Skip to content
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

[JENKINS-65287] Add an online missing help #48

Merged

Conversation

Onyimatics
Copy link
Contributor

@Onyimatics Onyimatics commented Apr 5, 2021

  • 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

  • Created help-changelog.html file to add a description of the scm step argument changelog

  • Created help-poll.html file to add a description of the scm step argument poll

  • Link to relevant issues in GitHub or Jira

  • JENKINS-65287
    @MarkEWaite @oleg-nenashev

@MarkEWaite
Copy link
Contributor

Thanks @Onyimatics and congratulations on submitting the first pull request from the SheCodeAfrica Contributhon.

I think that the help for the SCM step is better covered by the existing help that is stored in a "jelly" file. To learn more about how jelly files are used in Jenkins, refer to Jelly form controls.

screencapture-localhost-8080-jenkins-pipeline-syntax-2021-04-05-10_56_14-edit (2)

I believe that one of the two changes is being replaced by the jelly file and the other is appearing but the help from the jelly file is the better choice in that location.

Could you follow that same pattern and add a help file for the polling and changelog checkboxes? I believe that the file names for the help are derived from the names of the variables in Jenkins.

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Apr 5, 2021

@Onyimatics the idea I suggested to add online help for the "Include in polling?" item is available at MarkEWaite@c1f434a . I'm not 100% confident on the phrasing, but it matches with the variable name poll . You should be able to confirm it is visible from both the snippet generator and the online help.

A similar file would need to be added for changelog.

@Onyimatics
Copy link
Contributor Author

Thanks @MarkEWaite for your feedback. I'm really happy I successfully raised a PR.

This is the view from my end which showed me a help icon for checkout and then I added one for SCM. I'm wondering why it's double on your end.
Screenshot 2021-04-05 at 18 25 35

And this is how it looks like on the Snippet Generator Reference Page.
Screenshot 2021-04-05 at 18 47 00

However, I've taken your feedback, I will go ahead and discard the changes, and create the polling and changelog checkboxes using jelly file like you said.

Thanks so much.

@Onyimatics
Copy link
Contributor Author

@Onyimatics the idea I suggested to add online help for the "Include in polling?" item is available at MarkEWaite@c1f434a . I'm not 100% confident on the phrasing, but it matches with the variable name poll . You should be able to confirm it is visible from both the snippet generator and the online help.

A similar file would need to be added for changelog.

Alright @MarkEWaite
That's well noted. Thanks.

@MarkEWaite
Copy link
Contributor

Thanks for the comparison screenshot. I'm not sure what's different between your version and mine. I'm running the Google Chrome browser on Linux with Jenkins 2.277.1. As far as I can tell, you're running Jenkins 2.277.1 as well. I assume you're using macOS and Safari.

@Onyimatics
Copy link
Contributor Author

I'm actually using macOS and Google Chrome Browser.
I don't enjoy Safari Browser 😄

@@ -0,0 +1,3 @@
<div>
This is a special step that will checkout using the configuration options offered by any Pipeline-compatible SCM plugin.

Choose a reason for hiding this comment

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

Suggested change
This is a special step that will checkout using the configuration options offered by any Pipeline-compatible SCM plugin.
This is a special step that checks out the code using the configuration options offered by any Pipeline-compatible SCM plugin.

We prefer present tense to future in a case like this.
The proper verb for is "check out" rather than "checkout" and it needs an object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, @StackScribe that's well noted.
Thanks for the correction.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Onyimatics you can use the "Commit suggestion" to apply changes that others have suggested.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MarkEWaite Thanks, well noted.

@dwnusbaum
Copy link
Member

dwnusbaum commented Apr 5, 2021

Does this existing help file not show up in the UI anywhere? Maybe a bug with how SCMStep is abstract and GenericSCMStep is the concrete implementation?

@MarkEWaite
Copy link
Contributor

Does this existing help file not show up in the UI anywhere? Maybe a bug with how SCMStep is abstract and GenericSCMStep is the concrete implementation?

That help file is visible in the user interface. It is one of the '?' icons in the two that are shown in my screenshot.

@Onyimatics Onyimatics force-pushed the JENKINS-65287-scm-missing-online-help branch from 03eb71d to 61d73c0 Compare April 6, 2021 14:48
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

I've confirmed that the text is displayed in Jenkins both from the snippet generator and from the steps reference inside Jenkins.

Needs corrections to the "Include in changelog" description, since it is different than the polling setting.

Thanks very much!

Onyimatics and others added 2 commits April 6, 2021 22:29
…MStep/help-changelog.html

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
…MStep/help-changelog.html

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
@Onyimatics
Copy link
Contributor Author

I've confirmed that the text is displayed in Jenkins both from the snippet generator and from the steps reference inside Jenkins.

Needs corrections to the "Include in changelog" description, since it is different than the polling setting.

Thanks very much!

Thanks a lot for the correction.

MarkEWaite added a commit to MarkEWaite/docker-lfs that referenced this pull request Apr 6, 2021
@MarkEWaite
Copy link
Contributor

I've deployed a pre-release build in my test instance and verified it works as expected.

@rsandell rsandell merged commit dbbdb4e into jenkinsci:master Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants