Skip to content

Conversation

@DomGarguilo
Copy link
Member

fixes #3527

WIP - wanted to push my changes up early to see if this approach might be on the right track. I plan to add tests for this but any feedback including ideas pertaining to potential test cases is welcome.

@DomGarguilo DomGarguilo self-assigned this Jul 7, 2023
Copy link
Contributor

@keith-turner keith-turner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good start to me. I looked at the GSon builder to see if it had any options that might improve how deterministic the output is and I did not see anything.

@keith-turner
Copy link
Contributor

For the test case it may be worthwhile to have a few json strinsg that are not generated by gson so we can detect if the way gson generates json ever changes.

@keith-turner
Copy link
Contributor

For the test it would also be good to test comparison of json containing supersets and subsets of files to each other.

DomGarguilo and others added 2 commits July 10, 2023 11:48
Co-authored-by: Keith Turner <kturner@apache.org>
DomGarguilo and others added 2 commits July 10, 2023 13:58
…lectedFiles.java

Co-authored-by: Keith Turner <kturner@apache.org>
@DomGarguilo DomGarguilo marked this pull request as ready for review July 11, 2023 17:46
@DomGarguilo
Copy link
Member Author

@keith-turner, I tried to cover the test cases that you mentioned above.

@keith-turner
Copy link
Contributor

@keith-turner, I tried to cover the test cases that you mentioned above.

Those look really good. I made a few other comments. I think this is good to go after that. I need open an issue for the thing @dlmarion brought up or it will be forgotten.

Co-authored-by: Keith Turner <kturner@apache.org>
@DomGarguilo DomGarguilo merged commit 1ebe335 into apache:elasticity Jul 13, 2023
@DomGarguilo DomGarguilo deleted the selectedFilesSerializedComparable branch July 13, 2023 18:47
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Ensure serialzed verion of SelectedFiles is comparable and unit test this

4 participants