Page MenuHomeSoftware Heritage

Generate pom.xml files ourselves instead of using xmltodict.unparse.
ClosedPublic

Authored by vlorentz on Feb 12 2019, 6:29 PM.

Diff Detail

Repository
rDCIDX Object indexer
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Feb 12 2019, 6:29 PM
vlorentz updated this revision to Diff 3550.Feb 12 2019, 6:31 PM
  • More keys.
douardda requested changes to this revision.Feb 14 2019, 9:48 AM
douardda added a subscriber: douardda.

Please give a bit of context in your commit messages (in git). Like these 2 commits are dealing with tests for example. And 'More keys' is not a good commit message. Like 'fix a bug' is not a valid commit message, generally speaking.

This revision now requires changes to proceed.Feb 14 2019, 9:48 AM
vlorentz updated this revision to Diff 3579.Feb 14 2019, 10:49 AM

Reword commits:

  • Make tests generate pom.xml files themselves instead of using xmltodict.unparse.
  • Make xml_document_strategy generate elements 'repositories' and 'licenses'.

Please give a bit of context in your commit messages (in git). Like these 2 commits are dealing with tests for example. And 'More keys' is not a good commit message. Like 'fix a bug' is not a valid commit message, generally speaking.

Right, we have this documentation about it [1]

[1] https://wiki.softwareheritage.org/wiki/Git_style_guide

ardumont accepted this revision.Feb 14 2019, 11:11 AM

It'd be good to have tests around the xml_document_strategy since it's used for tests though.
for maintenance purposes, to make sure we can distinguish between corner cases in tests and ones in production code...

It'd be good to have tests around the xml_document_strategy since it's used for tests though.

How would you do that?

for maintenance purposes, to make sure we can distinguish between corner cases in tests and ones in production code...

What do you mean?

How would you do that?

test around the xml_document_strategy as usual (as many as the inlining of your branching conditions).
There is nothing specific about hypothesis on that function, so it's testable.

What do you mean?

If you have bugs in the current xml strategy used in tests, and the tests does strange things, we can't know what's wrong.
It could be the tests or the production code.

With tests on the strategy implementation, if something is off in the tests (due to modification in the code), we can more safely assume that it's the production code that is bugged and not the utilities around the tests.

vlorentz updated this revision to Diff 3586.Feb 14 2019, 1:46 PM
  • Add test for tree_to_xml.
douardda accepted this revision.Feb 14 2019, 2:56 PM
This revision is now accepted and ready to land.Feb 14 2019, 2:56 PM
This revision was landed with ongoing or failed builds.May 24 2019, 3:18 PM
This revision was automatically updated to reflect the committed changes.