Move parameter data types to the top #180
Conversation
src/main/java/org/jenkinsci/pipeline_steps_doc_generator/ToAsciiDoc.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jenkinsci/pipeline_steps_doc_generator/ToAsciiDoc.java
Show resolved
Hide resolved
| attrHelp.append("<ul>").append(typeDesc).append("</ul>"); | ||
| } | ||
| } catch (RuntimeException | Error ex) { | ||
| LOG.log(Level.WARNING, "Restricted description of attribute " |
There was a problem hiding this comment.
Capturing all errors always feels like a code smell to me. Which ones are ignorable and which ones are actually bad? This will mean that'll "randomly" have empty help and you'll have to go look at the logs to see why.
Its easier to maintain when you handle very specific errors so you know what kind of bad data it can expect
There was a problem hiding this comment.
Also, if you have a definite failure, maybe there's some parsing problem with the data that we can figure out here. I think this is a copy of what we have done for this sort of help before. So I'm sort of fine with skipping for no. But if we could find an example of a failure, that would be really interesting to look at to try to troubleshoot together.
|
I don't know the code base and am mobile but gave a few random feedback. Feel free to ignore. |
vihaanthora
force-pushed
the
param-details
branch
from
0303962
to
9d56182
Compare
vihaanthora
force-pushed
the
param-details
branch
from
9d56182
to
8fde043
Compare
|
Thanks for the feedback @halkeye 😄 |
|
@kwhetstone I checked the logs and was able to locate the error. I committed some changes to the same and the build is successful now. |
|
Just as something we should note here: let's make sure that we reference the JIRA ticket here in the comments of the PR. |
|
Sure thing, was doing the opposite till now 😅 |
| @@ -1,6 +1,18 @@ | |||
| package org.jenkinsci.pipeline_steps_doc_generator; | |||
|
|
|||
| import hudson.model.Descriptor; | |||
| import java.io.IOException; | |||
There was a problem hiding this comment.
Also wanted to reiterate here because of the same movements: normally when editing code, you don't want to move around dependency imports. A lot of projects will have a standard of imports which make it easier to quickly look at what's included in a project. I looked at Jenkins and didn't see anything
This PR moves the data types associated with each parameter (or further nested choices) to the top where the title of each is mentioned. This will allow users to view the data types without having to click the expand button or scroll through the help content.
The imports are organized so that there are no conflicts after merging #177.
Tagging the mentors: @kwhetstone @arpoch