Skip to content

Roundtrip test and some fixes to inconsistencies between the schema, examples, and JSONMetadataHandler#99

Closed
fedorov wants to merge 0 commit intoQIICR:masterfrom
fedorov:jsoncompare
Closed

Roundtrip test and some fixes to inconsistencies between the schema, examples, and JSONMetadataHandler#99
fedorov wants to merge 0 commit intoQIICR:masterfrom
fedorov:jsoncompare

Conversation

@fedorov
Copy link
Copy Markdown
Member

@fedorov fedorov commented Oct 16, 2016

Followup on #97.

jsoncompare is from https://github.com/ChannelIQ/jsoncompare, with ChannelIQ/jsoncompare#8 applied.

Comment thread apps/seg/Testing/CMakeLists.txt Outdated
)

add_test(NAME seg_meta_roundtrip
COMMAND python ${CMAKE_SOURCE_DIR}/util/comparejson.py
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@fedorov The test is failing, because this python file does not exist. Its name is jsoncompare.py

@che85
Copy link
Copy Markdown
Member

che85 commented Oct 19, 2016

@fedorov I suggest, that you first merge my PR after it passed all tests and then I will merge your changes because there will be conflicts....

@che85
Copy link
Copy Markdown
Member

che85 commented Oct 19, 2016

@fedorov Actually I can just create a new PR with a combination of those two because I already rebased your branch to mine.

What do you think?

@fedorov
Copy link
Copy Markdown
Member Author

fedorov commented Oct 19, 2016

sure, that is fine. I didn't look carefully into why the test fails, because I expected it to fail.

@che85
Copy link
Copy Markdown
Member

che85 commented Oct 19, 2016

Why did you expect it to fail?

@fedorov
Copy link
Copy Markdown
Member Author

fedorov commented Oct 19, 2016

Because not all of the attributes were populated on read, when I looked at the json converter produces.

Also, comparejson is a separate file I didn't commit. I added it now. I agree naming can be improved to be less confusing.

@che85
Copy link
Copy Markdown
Member

che85 commented Oct 20, 2016

@fedorov Can you force push your commits again. Somehow your commits disappeared after I pushed to your github (which actually should not be allowed)

@fedorov
Copy link
Copy Markdown
Member Author

fedorov commented Oct 20, 2016

Can you force push your commits again

done, sorry, overwhelmed by emails... No idea how it was possible for you to alter a repository under my account, you are not even listed as a collaborator under fedorov/dcmqi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants