-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat(3022): Mark workflow node as virtual if it corresponds to a virtual job #51
Conversation
annotations.forEach(({ annotation, fieldName, defaultValue }) => { | ||
const fieldValue = hoek.reach(jobs[name], annotation, { | ||
separator: '>', | ||
default: defaultValue | ||
}); | ||
|
||
if (fieldValue !== undefined) { | ||
m[fieldName] = fieldValue; |
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.
small suggestion: is using value
a little redundant for variable names?
annotations.forEach(({ annotation, fieldName, defaultValue }) => { | |
const fieldValue = hoek.reach(jobs[name], annotation, { | |
separator: '>', | |
default: defaultValue | |
}); | |
if (fieldValue !== undefined) { | |
m[fieldName] = fieldValue; | |
annotations.forEach(({ annotation, fieldName, default }) => { | |
const field = hoek.reach(jobs[name], annotation, { | |
separator: '>', | |
default | |
}); | |
if (field !== undefined) { | |
m[fieldName] = field; |
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.
Changing it to just field
may not make more sense as it represents value.
May be we could use just value
. Since we are using fieldName
for the key, fieldValue
seems more appropriate to represent the value.
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.
Also default
is a keyword and cannot be used as variable name.
fa50387
to
fd0dc65
Compare
🎉 This PR is included in version 4.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Context
Workflow graph node (Ex: implicit setup/teardown job of a stage) for an event is identified as virtual based on the presence/value of
screwdriver.cd/virtualJob
annotation in the corresponding job definition.Since the job definition reflects the latest configuration from
screwdriver.yaml
, referring to current annotations of the job may not be reliable to identify it as virtual for an event.We need a way to preserve whether the job is virtual or not when event is created.
Objective
Add a new attribute
virtual
in the workflow graphnode
to indicate whether it is virtual or not.References
screwdriver-cd/screwdriver#3022
License
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.