Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[gh-24311] Expand on documentation about jobs that are both parameterised and periodic #24384
[gh-24311] Expand on documentation about jobs that are both parameterised and periodic #24384
Changes from 3 commits
bcb344b
e57bbf4
4095fc5
4aaba64
961ef58
389ceb9
7a937bf
72bf22f
1053a53
0a1d152
cd239bd
eb53a62
ea4e464
3125809
fa0849e
e233bbe
bbdea38
4301c57
f278259
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I reorganized based on Daniel's L164 suggestion. The opening paragraph contains a lot of information, and I had to read it a couple of times to understand it all. I tried to reorganize into a basic sentence structure and numbered list so the content is easier to scan.
Style guide -> present tense, active voice, direct language (see https://github.com/hashicorp/team-nomad/blob/main/wiki/docs.md#sentence-structure)
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.
@Juanadelacuesta do you want to include this suggestion? After your most recent push, this suggestion would replace L157-164.
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.
whoa! I didn't notice these great
// comments
in the preview, because I didn't see that the little code block could scroll to the right; I only saw to theType
column. I'm not sure what to suggest for that... maybe say above this to "Scroll to the right for descriptions of each job."?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.
this seems a little buried under these Parameters. maybe move it up under Requirements?
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.
Since this periodic page doesn't talk about parameterized options at all, what about moving this line right before the
#
periodicRequirements
section?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.
It is a bit of a corner case, I didn't want to put it in the general description