-
Notifications
You must be signed in to change notification settings - Fork 308
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
revise: clarifies language around the allowed number of sections in tasks and workflows #598
revise: clarifies language around the allowed number of sections in tasks and workflows #598
Conversation
b50b258
to
52addde
Compare
b7c2883
to
df11c9c
Compare
@jdidion For clarifications like this, should we make them against 1.1.2? or against 1.1? |
@claymcleod this is a great PR, thanks! The only question is where to merge. Historically, our policy has been not to update a spec once it has been released. We recently modified this to allow patch releases, and have just released If we want to do another patch release to If it is decided that this branch should target In my opinion, we should get in the habit of making frequent patch releases (perhaps on a quarterly cadence, assuming there are any patches to make). So I suggest option 1 (targeting a new Would appreciate feedback on this from others before going forward. |
+1 to |
Yeah, I'm also in favor of |
This is based on @geoffjentry's suggestions on openwdl#598.
caea169
to
73b0033
Compare
This is based on @geoffjentry's suggestions on openwdl#598.
Moved to |
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.
Thanks @claymcleod. Overall I think this is great; however, I don't think we should be adding style recommendations here. Note that I haven't gone through the spec thoroughly to make sure I'm being consistent with this sentiment, but if there are places in the spec already where we're making stylistic recommendations I'd also advocate for moving those to a separate style guide.
This is based on @geoffjentry's suggestions on openwdl#598.
73b0033
to
5cab466
Compare
Previously, the number of sections within `task` and `workflow` blocks was implied rather than explicit. This commit introduces clear, concise language describing the number of sections allowed and whether each section is required/optional. Furhter, some housekeeping items were included to make the editted text more consistent. Also included in this commit (these changes were squashed): * revise: adds suggested ordering to elements This is based on @geoffjentry's suggestions on openwdl#598. * revise: adds `requirements` and `hints`, removes ordering recommendations Closes openwdl#594
5cab466
to
8000af5
Compare
@jdidion all comments have been addressed, the branch target has been updated to |
@claymcleod Regarding the target branch, maybe there was a misunderstanding. I think we agreed to target Otherwise it looks good and I'm prepared to merge it. |
Made these changes here #632 rather than trying to rebase |
Hey John, Apologies, I was planning on responding last week but then was out. In short, I had interpreted this comment:
To mean that it would be okay to merge into 1.2 if I was willing to do the work to update the sections to be compliant with 1.2. Originally, I had agreed with that, but then I found some time to just update it for 1.2. I'm okay with it going into either 1.1.2 or 1.2 provided that we make sure the right content gets on the right branch. In this case, the text you had merged in contains references to the |
This pull request clarifies the allowed number of member sections within
task
andworkflow
blocks.Rendered task and workflow sections.
It wasn't clear to me whether I should update bump the patch version number myself or whether the maintainers of the spec would bundle this change in with others—let me know if you'd like me to do that.
Closes #594.
Checklist