Remove namespace redundancy when generating json

[kkoehn]

This change adds an additional requirement for a namespace to be added to the namespace attribute in the json format:

• the parent node must not also have this namespace (prefix and href get both checked)

[MrSerth]

Awesome, what a simple fix! I actually spent some time to understand its cleverness and think it is suitable for our use case 👍

While checking the code, I also came up with a test for a bidirectional test -- it simply checks whether the XML -> JSON -> XML conversation (or JSON -> XML -> JSON) is performed without any change: bidirectional_converter_spec.rb.zip

I would propose adding it as well to our test suite -- just to be sure that a similar mistake cannot slip in again.

[kkoehn]

Awesome, what a simple fix! I actually spent some time to understand its cleverness and think it is suitable for our use case 👍

Thanks for the feedback!

While checking the code, I also came up with a test for a bidirectional test -- it simply checks whether the XML -> JSON -> XML conversation (or JSON -> XML -> JSON) is performed without any change: bidirectional_converter_spec.rb.zip

I would propose adding it as well to our test suite -- just to be sure that a similar mistake cannot slip in again.

Do you think it fits in the scope of this PR? I don't see any direct ties.

[MrSerth]

Do you think it fits in the scope of this PR? I don't see any direct ties.

Somewhat. The tests I provided wouldn't pass without your change, so that they cannot be added earlier. However, as soon as this PR is merged, it can be added at any time.

I thought it would fit somewhat in the scope of this PR, since this PR resolves an issue that was previously preventing the bidirectional conversion to function correctly.

Nevertheless, I am also fine to merge this PR and just open another one with my changes 😁

[kkoehn]

Somewhat. The tests I provided wouldn't pass without your change, so that they cannot be added earlier. However, as soon as this PR is merged, it can be added at any time.

Weird - I tried adding the test directly to main and it works fine for me 😕

[codecov]

Codecov Report

Merging #52 (f0f59b0) into main (a8a44a0) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          315       315           
=========================================
  Hits           315       315           

Files	Coverage Δ
lib/dachsfisch/xml2_json_converter.rb	100.00% <100.00%> (ø)
spec/support/examples.rb	100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[kkoehn]

Nonetheless, feel free to commit it to this PR to reduce the overhead

[MrSerth]

Weird - I tried adding the test directly to main and it works fine for me 😕

You're right, I see that as well. Obviously, the issue is with the be_equivalent_to, which smartly checks for the namespace (this is the only case it would break).

I'll merge this PR and will open another one with my changes soon. Then, we can continue the discussion there.