Reset quality gate, no information about who, when and why#3221
Reset quality gate, no information about who, when and why#3221uhafner merged 11 commits intojenkinsci:mainfrom
Conversation
…d DateTimeFormatter
|
Please review the changes. @uhafner |
uhafner
left a comment
There was a problem hiding this comment.
Thanks for fixing this issue.
Do you think it is possible, to show the message immediately after the button disappears? Otherwise one needs to reload the page...
| </j:if> | ||
| <j:if test="${s.qualityGateReset}"> | ||
| <li style="font-style: italic; color: #666;"> | ||
| Quality gate reset by ${s.resetBy} on ${s.resetTimestamp} |
There was a problem hiding this comment.
Can you use Luxon (<script type="text/javascript" src="${resURL}/plugin/data-tables-api/js/luxon.min.js"/>) to render the date?
There was a problem hiding this comment.
Or in Jenkins core we have a Java utility that shows a timestamp in a user friendly manner: Util.getTimeSpanString.
| </button> | ||
| </j:if> | ||
| <j:if test="${s.qualityGateReset}"> | ||
| <li style="font-style: italic; color: #666;"> |
There was a problem hiding this comment.
Never use style, use one of the styles in our design library (css class).
https://weekly.ci.jenkins.io/design-library/colors/
Probably the best color is secondary text.
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ResetReferenceAction.java
Show resolved
Hide resolved
| this.id = id; | ||
| this.userId = userId; | ||
| this.timestamp = timestamp; | ||
| this.reason = reason == null ? "" : reason; |
There was a problem hiding this comment.
This can't be null
| this.reason = reason == null ? "" : reason; | |
| this.reason = reason; |
| private final String id; | ||
| private final String userId; | ||
| private final long timestamp; | ||
| private final String reason; |
There was a problem hiding this comment.
Are you planning to add support for the reason? Otherwise remove this field.
| User user = User.getById(userId, false); | ||
| return user != null ? user.getFullName() : userId; | ||
| } | ||
| catch (IllegalStateException e) { |
| </j:if> | ||
| <j:if test="${s.qualityGateReset}"> | ||
| <li style="font-style: italic; color: #666;"> | ||
| Quality gate reset by ${s.resetBy} on ${s.resetTimestamp} |
There was a problem hiding this comment.
Can we have a link to the user if available?
plugin/src/main/java/io/jenkins/plugins/analysis/core/model/ResetReferenceAction.java
Show resolved
Hide resolved
| /** | ||
| * Returns the optional reason for resetting the quality gate. | ||
| * | ||
| * @return the reason, or an empty string if no reason was provided | ||
| */ | ||
| public String getReason() { | ||
| return reason; | ||
| } | ||
|
|
||
| /** | ||
| * Returns whether a reason was provided for the reset. | ||
| * | ||
| * @return {@code true} if a reason was provided, {@code false} otherwise | ||
| */ | ||
| public boolean hasReason() { | ||
| return !reason.isEmpty(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Remove if you have no plans to support this in the UI
I have tried to implement your this request. But can't implement properly, face many issues, so I revert the changes. Please review the other suggested changes which I have implemented accordingly. @uhafner |
d01fd81 to
3d877e2
Compare
plugin/src/main/resources/io/jenkins/plugins/analysis/core/model/ResultAction/summary.jelly
Outdated
Show resolved
Hide resolved
plugin/src/main/resources/io/jenkins/plugins/analysis/core/model/ResultAction/summary.jelly
Outdated
Show resolved
Hide resolved
8f0b7fb to
af98d13
Compare
|
I did a mistake, I force-pushed a commit without updating my branch locally, as a result your commit disappeared. I am very sorry for that. I have implemented all the requested changes. But now I’m seeing a few test failures, don't know why. I don't think these are related to my changes. What should I do now? @uhafner |
|
Tipp: you should never use a force push or rebase when someone already did a code review, otherwise the annotations do not get resolved. And the reviewer needs to delete the changed branch and checkout again... I have no idea why the artifacts are not found, I re-triggered the builds. |
Reset quality gate, no information about who, when and why
Fixes #2879
Testing done
Submitter checklist