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

Update readme content roles description #75

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Komodoro
Copy link

@Komodoro Komodoro commented Jun 9, 2021

A much more informative way to describe the roles.
This description is available inline of the html while hovering the respective role attribute.

This is not an urgent call but it seems that it is missing from the documentation. I felt an urge to help others by making this available. Using this plugin on a daily basis, and having this properly documented in the README file of the project, the respective table can provide assistance to others understand better what they are selecting instead of just hovering over the values in the small text box that the plugin has. Since hovering details only show info for a little nick of time this is a much more a intuitive approach, as well as it help define strategies to apply this
Probably not a best practice since there could be a lot of places to have this table on, but at least it will help some folks who want to have this documented somehow.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main 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

José Pedro Pereira added 2 commits June 9, 2021 16:33
A much more informative way to describe the roles being in the README
file of the project helping others to understand better what they are
selecting instead of just hovering over thje values in the small text
box that the plugin has. And since hovering details only show them for a
little nick of time this is a much more intuitive approach
@AbhyudayaSharma AbhyudayaSharma added documentation Improvements or additions to documentation enhancement New feature or request and removed enhancement New feature or request labels Jun 9, 2021
@AbhyudayaSharma AbhyudayaSharma self-requested a review June 9, 2021 16:50
@Komodoro
Copy link
Author

Komodoro commented Jun 9, 2021

@AbhyudayaSharma maybe this should be in the docs folder. Let me know if this makes more sense.

Copy link
Member

@AbhyudayaSharma AbhyudayaSharma left a comment

Choose a reason for hiding this comment

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

@AbhyudayaSharma maybe this should be in the docs folder. Let me know if this makes more sense.

@Komodoro Thanks a lot for the pull request. I would suggest you to please add this information here: https://github.com/jenkins-infra/jenkins.io/blob/master/content/doc/book/security/access-control/permissions.adoc. There are several other plugins like role-strategy and matrix-auth that can make use of this information. Having a shared documentation that can be updated according to changes in Jenkins core would be better. We can then add a link to that page from the docs of this plugin.

You can refer to https://github.com/powerman/asciidoc-cheatsheet#tables for converting the table to asciidoc which is used by Jenkins.io.

@AbhyudayaSharma
Copy link
Member

Also, please ignore the failing build. This is a problem with ci.jenkins.io, not with the pull request.

@Komodoro
Copy link
Author

Sorry... Had some time off from work and I have this on my work PC.
Your plugin is saving a lot of people.
I'll update ASAP.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants