Skip to content
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

[FIX]In some cases the merged XML was missing blocks or fields #15

Merged
merged 1 commit into from
Jun 8, 2018

Conversation

sferket
Copy link
Contributor

@sferket sferket commented Jun 8, 2018

No description provided.

Copy link
Owner

@bobslee bobslee left a comment

Choose a reason for hiding this comment

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

Hi Stefaan,
Thanks for your nice change!
Before I accept your PR, I have the following question.

Regarding the following condition, the images and annotations won't be merged ?!

if tag not in ['annotation', 'image']:

@sferket
Copy link
Contributor Author

sferket commented Jun 8, 2018

Bob,

Both where problematic in the past. This is the reason we took them out in the old module (the Java one).
I would prefer taking them out now (just like we did in the past) and pick the implementation of annotation and image merge up in a later stage.

@bobslee
Copy link
Owner

bobslee commented Jun 8, 2018

Allright, that seems fair enough to me (for the time being).
I created issue #16 to handle those image and annotation elements in the merge.

@bobslee bobslee merged commit 6b9be15 into bobslee:master Jun 8, 2018
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