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

Extend XMLStreamWriter validation test coverage #191

Merged
merged 6 commits into from
Feb 5, 2024

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Jan 13, 2024

@cowtowncoder this is a preparation for the fix of #179:

  • Assert specific validation error messages to get more certainty that the expected error occurred
  • Extend XMLStreamWriter validation test coverage by checking the writer validation wherever we test the reader validation

validation wherever we test the reader validation
…verbose"' not thrown from RepairingNsStreamWriter when validating against a DTD schema
@cowtowncoder
Copy link
Member

Would be happy to merge this. Since there are non-test changes, one thing we need to have CLA (if not received yet, apologies if I missed it). One from Jackson repo:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

is one we use; it is usually easiest to print, fill & sign, scan/photo, email to cla at fasterxml dot com.

Once I have CLA it's good for all future contributions.

Looking forward to merging this, thank you!

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 15, 2024

we need to have CLA

I asked our legal team for an approval.

@cowtowncoder
Copy link
Member

Thank you @ppalaga .

@cowtowncoder
Copy link
Member

Since the only remaining changes here are test improvements, I can merge without CLA -- will do so.
(CLA needed for actual non-test code changes, just not test code)

@cowtowncoder cowtowncoder merged commit 5a03810 into FasterXML:master Feb 5, 2024
4 of 5 checks passed
@ppalaga
Copy link
Contributor Author

ppalaga commented Feb 28, 2024

@cowtowncoder our Legal team did not approve the CLA you pointed me to. The main concern lies in the broad scope of the copyright license, particularly with the request for joint ownership.
I wonder if I may let them review the corporate CLA?

@cowtowncoder
Copy link
Member

Yes CCLA makes sense then. Have had a few company submit that, including some of FAANGs; no real difference for us.

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