Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue-45 exop #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

the-red-paintings
Copy link
Contributor

Checklist

  • This commits targets only one specific issue
  • Docs have been added or updated

Issues

Refs #45

@the-red-paintings
Copy link
Contributor Author

@sobolevn, please review

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you work! Can you please explain what has changed in the library?
And what are other solutions to fix the upgrade?

@@ -8,7 +8,10 @@ defmodule Kira.Projects.Commands.QueueIssue do
alias Kira.Projects.Queries.IssueQueries
alias Kira.Repo

parameter(:issue_uid, type: :integer)
parameter(:project_uid, type: :integer, required: false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need all these params? What has changed in the library?

@@ -8,7 +8,10 @@ defmodule Kira.Projects.Commands.QueueIssue do
alias Kira.Projects.Queries.IssueQueries
alias Kira.Repo

parameter(:issue_uid, type: :integer)
parameter(:project_uid, type: :integer, required: false)
parameter(:issue_uid, type: :integer, required: true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only issue_uid is used directly. Other params are unused.

@@ -11,7 +11,10 @@ defmodule Kira.Projects.Services.NoteCommands.QueueIssue do
# TODO: move to configuration
@bot_username "@kira-bot"

parameter(:note_text, type: :string)
parameter(:project_uid, type: :integer, required: false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here. Why do we need al of them when we only use note_text?

@the-red-paintings
Copy link
Contributor Author

@the-red-paintings
Copy link
Contributor Author

the-red-paintings commented Jun 26, 2019

Since 1.3 exop had breaking changes for run/1 and now method only can access to variables defined in contract

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants