-
Notifications
You must be signed in to change notification settings - Fork 490
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
Ordering subfields while displaying dataset version differences #10969
Ordering subfields while displaying dataset version differences #10969
Conversation
@luddaniel thanks for the PR! Any idea why we are seeing has this error?
|
It's very unlikely, for anything in this PR to cause that test to fail. It is known to be a little flaky - let me take a quick look. |
(seeing how Jenkins tests did succeed once earlier today, on the same branch minus the release note, I am not too worried about it) |
It has just passed again (build #3). Somebody, merge it please before it breaks again. |
Ha, well, I did put it in our new "ready for triage" column which we now look at as a team on Tuesdays, I believe. It looks like there's the potential for merge conflicts with this PR: |
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 also checked the possibility of a merge conflict but did see any with code or tests
@luddaniel After pushing the fix to Internal and testing it, I noticed that the issue is still observed. See screenshots along with test steps below: Test Steps:
|
Hi @ofahimIQSS Maybe my eyes trick me but it looks to work as expected :) |
Merging PR - Testing Passed. @luddaniel Thank you! |
What this PR does / why we need it:
Ordering subfields while displaying dataset version differences
Which issue(s) this PR closes:
Special notes for your reviewer:
This code looks NullPointerException safe so I didn't overprotect it.
Suggestions on how to test this:
Create multiple versions by modifying the subfields each time and checking the operation with the comparison pop-up.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Before:
After:
Is there a release notes update needed for this change?:
yes