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

Enhanced getFillItems() method in HttpMode class #143

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

Conversation

iqbalrohail
Copy link
Contributor

  • done this to populate a ListBoxModel with HttpMode enum values

  • This change will likely improve the user experience by providing a better list of options for selecting the HTTP mode in Jenkins build steps.

  • 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

done this to populate a ListBoxModel with HttpMode enum values
@iqbalrohail iqbalrohail requested a review from a team as a code owner April 15, 2023 11:33
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.

This is an interesting first step, but I think that it is not providing enough value to the user to justify the change. The value of httpMode.toString() is the same as the value of httpMode.name(). Thus, the additional parameter does not provide any better information for the user than the existing parameter.

If you'd like to continue on this path, then the second argument should provide more information for the user than the first argument does. It might be a combination of the httpMode and a brief description of that mode possibly using some portion of the RFC-9110 method definitions or the Wikipedia request method descriptions.

Remember that the second parameter is the text that will be displayed to the user in the dropdown list that is presented. I think it should include the httpMode.name() as the first text in the dropdown list so that the selected method is the first text read by the user.. Additional explanation should come after the name of the method.

@MarkEWaite
Copy link
Contributor

@iqbalrohail would you like to continue with this pull request or should I close it?

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

Successfully merging this pull request may close these issues.

2 participants