Skip to content

[JENKINS-65420] Added missing online helps#139

Merged
car-roll merged 34 commits intojenkinsci:masterfrom
ciradu2204:add-online-help
May 5, 2022
Merged

[JENKINS-65420] Added missing online helps#139
car-roll merged 34 commits intojenkinsci:masterfrom
ciradu2204:add-online-help

Conversation

@ciradu2204
Copy link
Copy Markdown
Contributor

@ciradu2204 ciradu2204 commented Apr 21, 2021

@MarkEWaite @StackScribe @kwhetstone

More details about the issue -> JENKINS-65420

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@@ -0,0 +1,3 @@
<div>
The relative path of the directory to the workspace directory.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am not sure on this one, I looked at the Java class here


the comment there is Temporarily changes the working directory..

I think it's more to change the folder to work in.

I do a suggestion but I am not sure on my English :), feel free to change it.

Suggested change
The relative path of the directory to the workspace directory.
The relative path in workspace to the folder to use as a new working directory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you @aHenryJard. What about this: The relative path of the folder to the workspace to use as a new working directory?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cynthia, I like your rewrite, but how about going back to "directory" rather than "folder"? I know that the terms are generally used to be synonymous but Jenkins does have a folders feature so it could be ambiguous. Also, how about "directory in the workspace"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, thank you @StackScribe. I have made the changes.

Comment thread src/main/resources/org/jenkinsci/plugins/workflow/steps/ToolStep/help-name.html Outdated
Comment thread src/main/resources/org/jenkinsci/plugins/workflow/steps/ToolStep/help-type.html Outdated
@@ -0,0 +1,3 @@
<div>
A message to write to the console output.
</div> No newline at end of file
Copy link
Copy Markdown
Contributor

@bitwiseman bitwiseman Apr 23, 2021

Choose a reason for hiding this comment

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

I think we want all the files to end with a newline.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bitwiseman. I have accepted your changes

ciradu2204 and others added 6 commits April 23, 2021 21:28
…ep/help-type.html

Co-authored-by: Liam Newman <bitwiseman@gmail.com>
…ileStep/help-file.html

Co-authored-by: Liam Newman <bitwiseman@gmail.com>
…ep/help-name.html

Co-authored-by: Liam Newman <bitwiseman@gmail.com>
@ciradu2204 ciradu2204 changed the title [JENKINS-65420] Added missing online help [JENKINS-65420] Added missing online helps Apr 26, 2021
@@ -0,0 +1,3 @@
<div>
Relative path of a file within the workspace.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to the Pushd documentation, do you want to want to make this match terminology by using directory here? I think it makes sense to match but want to confirm /cc @StackScribe

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kwhetstone, I thought directory in only used to specify a folder not a file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure of the context here -- does this parameter specify the directory where the file will be written or the path (including he file name) of the file. That would determine the proper wording here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@StackScribe, the path of the file.

@@ -0,0 +1,3 @@
<div>
The name of the tool. The tool name must be pre-configured in Jenkins under Manage Jenkins → Global Tool Configuration.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add hyperlink to the management page? Or highlight the Web UI elements via italic or whatsover?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @oleg-nenashev, I have made it italic

ciradu2204 and others added 3 commits April 27, 2021 10:03
Copy link
Copy Markdown

@aHenryJard aHenryJard left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Copy link
Copy Markdown
Contributor

@StackScribe StackScribe left a comment

Choose a reason for hiding this comment

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

Nicely done!

Copy link
Copy Markdown
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Approved once suggestions by @StackScribe are integrated

ciradu2204 and others added 3 commits April 28, 2021 09:55
…ep/help-message.html

Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
…s/stash/StashStep/help-allowEmpty.html

Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
…tep/help-message.html

Co-authored-by: Meg McRoberts <mmcroberts@cloudbees.com>
Copy link
Copy Markdown
Member

@alecharp alecharp left a comment

Choose a reason for hiding this comment

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

I'd prefer to see the two suggestions incorporated into the PR but I'm not blocking it either.

Comment thread src/main/resources/org/jenkinsci/plugins/workflow/steps/PushdStep/help-path.html Outdated
Comment thread src/main/resources/org/jenkinsci/plugins/workflow/steps/ToolStep/help-name.html Outdated
ciradu2204 and others added 2 commits April 28, 2021 15:40
…ep/help-name.html

Co-authored-by: Adrien Lecharpentier <adrien.lecharpentier@gmail.com>
…tep/help-path.html

Co-authored-by: Adrien Lecharpentier <adrien.lecharpentier@gmail.com>
Comment thread src/main/resources/org/jenkinsci/plugins/workflow/steps/ToolStep/help-type.html Outdated
ciradu2204 and others added 2 commits April 28, 2021 16:19
…s/stash/StashStep/help-allowEmpty.html

Co-authored-by: Ramon Leon <manuelramonleonjimenez@gmail.com>
…ep/help-type.html

Co-authored-by: Ramon Leon <manuelramonleonjimenez@gmail.com>
@ciradu2204
Copy link
Copy Markdown
Contributor Author

ciradu2204 commented Apr 30, 2021

Hi @alecharp @bitwiseman @kwhetstone @StackScribe, I have worked on your reviews. If there are no other improvements you want me to make, it would be amazing if you accept this PR and merge it. Thank you!

@car-roll car-roll merged commit f0e6639 into jenkinsci:master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants