Fix namespace handling regression #403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lblackstone
changed the title
lblackstone
force-pushed
the
lblackstone/namespace-bug
branch
from
f236702 to
5138a01
Compare
lblackstone
changed the title
…ources that could conflict with the dynamic client for that resource. All of the namespace handling has now been deferred to the client package rather than the provider.
lblackstone
force-pushed
the
lblackstone/namespace-bug
branch
from
5138a01 to
d2000fb
Compare
lblackstone
changed the title
hausdorff
reviewed
Feb 6, 2019
|
@hausdorff I added that at some point deep in the middle of trying to get things working again. Now that the client package is cleaned up, the extra logic in the provider was redundant/not quite right. |
hausdorff
suggested changes
Feb 6, 2019
There was a problem hiding this comment.
Ok, so:
- This code is subtle but does seem to replace objects when
"default"->""namespace. - I actually forget (and can't figure out) why
diff.gocorrectly does not register""->"default"namespace does not report replacement, as our tests clearly show it does.
Overall I'm kind of scared of the Diff code.
lblackstone
changed the title
|
@hausdorff I fixed the regression and expanded the new test to include both the cases you mentioned. |
hausdorff
approved these changes
Feb 6, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Due to recent refactoring, the provider was setting namespaces on resources
that could conflict with the dynamic client for that resource. All of the namespace
handling has now been deferred to the client package rather than the provider.
Fixes #400