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

initial placeholder #105

Merged
merged 6 commits into from Oct 14, 2017

Conversation

Projects
None yet
3 participants
@zheng-wang
Contributor

zheng-wang commented Oct 14, 2017

No description provided.

@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2017

Coverage Status

Coverage increased (+0.04%) to 92.417% when pulling cd8e1f5 on zheng-wang:master into 038b8b9 on xmlunit:master.

@bodewig

This comment has been minimized.

Member

bodewig commented Oct 14, 2017

Many thanks!

I'm deliberately not using author tags, would it be OK with you if I removed the "Created by" lines? You will certainly be credited in the release notes.

@zheng-wang

This comment has been minimized.

Contributor

zheng-wang commented Oct 14, 2017

No problem. I have removed the author tags which were created by my IDE template.

@coveralls

This comment has been minimized.

coveralls commented Oct 14, 2017

Coverage Status

Coverage increased (+0.04%) to 92.417% when pulling 581510f on zheng-wang:master into 038b8b9 on xmlunit:master.

@bodewig bodewig merged commit 0297044 into xmlunit:master Oct 14, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.04%) to 92.417%
Details

bodewig added a commit that referenced this pull request Oct 14, 2017

@bodewig

This comment has been minimized.

Member

bodewig commented Oct 14, 2017

It would be good to add something to the user guide, I'll try to find some time to start this.

Many thanks again.

@bodewig

This comment has been minimized.

Member

bodewig commented Oct 14, 2017

I've just published 2.5.1-SNAPSHOT jars of the new module and updated the public javadocs at www.xmlunit.org

@zheng-wang

This comment has been minimized.

Contributor

zheng-wang commented Oct 15, 2017

Cool, thanks.

@bodewig

This comment has been minimized.

Member

bodewig commented Oct 15, 2017

I've reworked PlaceholderDifferenceEvaluator so it now also works for attributes and added infrastructure that should allow new placeholder keywords to become supported more easily. It would be good if you could take a look to ensure I've been moving into the direction you wanted it to go.

@zheng-wang

This comment has been minimized.

Contributor

zheng-wang commented Oct 16, 2017

Great job Stefen! Now the code is less prototyped and more frameworkized. The separation of concerns (placeholder specific logic goes to different classes) is a must go path I believe.

The complexity of the code increased a little bit but I can't think of a simpler way to achieve the separation of concerns. Eventually it should not be a problem to me as the main direction I have is requirement (from API user's perspective) which is reflected in the unit tests. : )

@bodewig

This comment has been minimized.

Member

bodewig commented Oct 16, 2017

OK, thanks.

I think I may have gone a bit too far and thus the API may be too limited. Right now the keyword between ${xmlunit. and } has to be a constant. I can easily envision something like ${xmlunit.matches(some-regex)} which would require to expand the placeholder language.

But we can fix that once we get there :-)

@bodewig bodewig added this to the 2.6.0 milestone Oct 20, 2017

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