-
Notifications
You must be signed in to change notification settings - Fork 12
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
Added ports to the scatter and enable usage of variables into the scatter #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine. I just have a couple of questions here.
if (!(config instanceof Action)) { | ||
config.canHavePorts = false; | ||
} else { | ||
if (config instanceof Action) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still unable to create groups from actions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@voidest currently it looks like there are no sense to create groups from actions. Let me leave it for a while.
test/model/GroupTest.js
Outdated
@@ -37,7 +37,7 @@ describe('model/Group', () => { | |||
}); | |||
|
|||
it('throws error when try to create with ports', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename/remove this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the notisance!
I will correct the name of this test
5bf2819
to
81ba15a
Compare
General idea
Since PR#20 was been merged it become possible to have a connections and ports in the scatter (if/while). This possibility make us able to propose solution for the following issues: #9 , #11 , #15 , and rewrite temporary fix for #16 , #21 . Currently pipeline-builder does not support the inputs in scatter(if/while) and does not show the scatter iteration item as a port. This pull request implements this features and hopefully fixes some bugs ( #15 , #16 , #21 ).
In this PR it will look like:
Changes
Scripts affected