Skip to content

Allows SchemaElement instance to use import namespace as targetNamesp…#1095

Closed
ndeniche wants to merge 2 commits intovpulim:masterfrom
ndeniche:ndeniche_targetNamespace
Closed

Allows SchemaElement instance to use import namespace as targetNamesp…#1095
ndeniche wants to merge 2 commits intovpulim:masterfrom
ndeniche:ndeniche_targetNamespace

Conversation

@ndeniche
Copy link

@ndeniche ndeniche commented Nov 7, 2019

Allows SchemaElement instance to use import namespace as targetNamespace when the attribute is not set.

Prevents the following error message when schema elements do not include targetNamespace attribute:

Target-Namespace "undefined" already in use by another Schema!

If targetNamespace is declared by an import is used in a different schema, it overrides the value for the targetNamespace key.

…ace when the attribute is not set.

### Allows SchemaElement instance to use import namespace as targetNamespace when the attribute is not set.

Prevents the following error message when schema elements do not include targetNamespace attribute:

> Target-Namespace "undefined" already in use by another Schema!
@coveralls
Copy link

coveralls commented Nov 7, 2019

Coverage Status

Coverage decreased (-0.5%) to 94.531% when pulling 79daeb4 on ndeniche:ndeniche_targetNamespace into af93a6f on vpulim:master.

@jsdevel
Copy link
Collaborator

jsdevel commented Nov 14, 2019

@ndeniche please fix the build and bring coverage back up.

@ndeniche
Copy link
Author

Same as the other added feature. All of the new code is covered. Should I improve coverage by adding tests to other features?

@jsdevel
Copy link
Collaborator

jsdevel commented Nov 18, 2019

@ndeniche if coverage drops on this build, that means that added code has decreased it right?

@barboni
Copy link
Contributor

barboni commented Dec 11, 2019

Screenshot 2019-12-11 at 10 42 37
This really doesn't make much sense. The overall coverage dropped, but the coverage of each file increased?! 😲

@sfariaNG
Copy link

Was the test coverage on this resolved? Can it be merged soon?

@jsdevel
Copy link
Collaborator

jsdevel commented Feb 10, 2020

@barboni it could be the case that this fix now makes other code unreachable: code who's past assumptions are no longer valid with this fix. can you look into it?

@nicolasjolinUQAM
Copy link

nicolasjolinUQAM commented Jul 21, 2020

I have the same problem as ndeniche and I don't have control over the WSDL. A fix would be appreciated.

@jsdevel
Copy link
Collaborator

jsdevel commented Jul 21, 2020

as long as coverage doesn't dip, we can merge the fix.

@nicolasjolinUQAM
Copy link

It would be kindly appreciated !

@jsdevel
Copy link
Collaborator

jsdevel commented Jul 21, 2020

well, someone has to add a test. feel free to plagaraize this in a new pr with a test

@smokhov
Copy link
Contributor

smokhov commented Jan 12, 2025

@w666 -- it seems this just needs a rebase for the recent checks to run

@smokhov
Copy link
Contributor

smokhov commented Jun 14, 2025

@w666 -- this PR can now simply be closed, since its superseding PR got merged. Thank you.

@w666 w666 self-requested a review July 9, 2025 08:41
@w666
Copy link
Collaborator

w666 commented Jul 9, 2025

Closing

@w666 w666 closed this Jul 9, 2025
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.

8 participants