-
Notifications
You must be signed in to change notification settings - Fork 4
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
Issue 50925: Refinements to peptide outlier heat map #943
Conversation
… targetedms-qcSummaryHistory.view
…he total number in the folder
…& reduce the number of boxes in the legend
Testing notes. Some are preexisting behaviors, some are new:
I imagine those will be small changes, but I'll hold my code review until you've had a chance to push fixes. |
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.
See my previously posted comments for some testing feedback.
…the new sort order
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.
A couple of small requests but no need for further review.
for (SampleFileInfo sampleFileInfo : sampleFileInfos) | ||
{ | ||
DateFormat format = DateFormat.getDateInstance(); | ||
String acquiredTime = format.format(sampleFileInfo.getAcquiredTime()); |
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.
Why do we format this as a string and then immediately parse it back to a Date?
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 was doing this to get the date only part. Thanks for the pointer, replaced it with DateUtil.getDateOnly
{ | ||
for (SampleFileInfo sampleFileInfo : sampleFileInfos) | ||
{ | ||
DateFormat format = DateFormat.getDateInstance(); |
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.
We should use DateUtil.parseDate().
Rationale
Following refinements are made in this PR-
Related Pull Requests
Changes