-
Notifications
You must be signed in to change notification settings - Fork 55
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
Add job start and duration time to job page #458
Add job start and duration time to job page #458
Conversation
57c58e1
to
3a508f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I've pushed a few changes so it doesn't take up as much room, let me know what you think.
I've left some inline comments or TODOs in the code where it could be improved
FYI I'll be away for ~2 weeks or so in a few hours and I won't merge this before leaving as I don't want to be not around in case there's an issue.
|
||
SimpleDateFormat sdf = new SimpleDateFormat("d MMM, hh:mm", Locale.getDefault()); | ||
// TODO support user timezones from UserProperty | ||
sdf.setTimeZone(TimeZone.getDefault()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users are able to set their own timezone in their user configuration, it would be good to support this, and also the Jenkins timezone property won't be supported by this core Java feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed I would have much rather passed epoch into RunInfo
, also respecting a users preference for clock notation(12/24) but that information isn't bubbled up to this plugin yet..
// TODO support user timezones from UserProperty | ||
sdf.setTimeZone(TimeZone.getDefault()); | ||
// TODO display full duration on hover but continue shortened for display | ||
this.startTime = sdf.format(new Date(run.getStartTimeInMillis())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there was too much info there before, I've shortened it but it would be good to show the full version on hover (ideally client side formatted I just didn't have much time)
<p>{run.displayName}</p> | ||
</a> | ||
<div className="PWGx-Start"> | ||
<span>{run.startTime} - {jobStatus === "in_progress" ? "Running..." : run.duration}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...o/jenkins/plugins/pipelinegraphview/multipipelinegraphview/MultiPipelineGraphViewAction.java
Fixed
Show fixed
Hide fixed
Any help needed to move this forward? |
Closing due to age. Will revisit |
This PR adds the job start time and job duration to the table view on the jobs page along with minor formatting to make it cleaner and easier to use:
For the ID column custom displayNames are accounted for w ellipsis cutoff. This PR addresses issue #449. Note I made some minor cleanups from the screenshot in the issue. Here is the final view:
Submitter checklist