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

Factorise parsing and conversion #174

Merged
merged 10 commits into from
Jul 28, 2021
Merged

Conversation

tardyp
Copy link
Contributor

@tardyp tardyp commented Jun 25, 2021

The example dir looks very confusing with lots of similar code, and the CLI is actually using it.

This PR provide factorization, so that it is easier to write a custom converter without having to copy paste too much boileplate.

The tests show that the conversion is not fully stable.

fixing that is out of scope of this PR, and requires more investigation

@tardyp tardyp force-pushed the factorise_parsing branch from 8ecd38b to f657272 Compare June 25, 2021 14:54
@tardyp
Copy link
Contributor Author

tardyp commented Jun 28, 2021

I did not realize this is not using pytest. I would suggest to use it as it is defacto standard, and has a lot of nice functionality to improve testability

tardyp added 2 commits July 21, 2021 09:40

Verified

This commit was signed with the committer’s verified signature. The key has expired.
lukekarrys Luke Karrys
a lot of the examples are actually the same code and the cli is using it
We factorize it so that it is easier to make some more converters

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
instead of having the same boilerplate in all the convertors,
we just factorize in a single function that can output any format
according to the file extension

The tests show that the conversion is not fully stable.

fixing that is out of scope of this commit, and requires more investigation

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
@tardyp tardyp force-pushed the factorise_parsing branch from f657272 to 56d1616 Compare July 21, 2021 07:40
tardyp added 2 commits July 21, 2021 13:21
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
@tardyp tardyp force-pushed the factorise_parsing branch from 22d8865 to 8f72d84 Compare July 21, 2021 12:08
tardyp and others added 3 commits July 22, 2021 11:44
…ll names

Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Signed-off-by: Pierre Tardy <tardyp@gmail.com>
Fix some test by regenerating the expected data

Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
@tardyp tardyp force-pushed the factorise_parsing branch from 8f72d84 to 49cab4b Compare July 22, 2021 09:44
Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
@tardyp tardyp force-pushed the factorise_parsing branch from abafce0 to e27cc07 Compare July 22, 2021 14:04
previous version of the test was unstable to directory order
the first file found was used, and some more simple files
have good conversion while more complex are failing

Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
@tardyp tardyp force-pushed the factorise_parsing branch from e27cc07 to 3eda4f9 Compare July 22, 2021 19:37
Signed-off-by: Pierre Tardy <pierre.tardy@renault.com>
@tardyp tardyp force-pushed the factorise_parsing branch from e8d774f to 3efab86 Compare July 26, 2021 19:49
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you ++

@pombredanne pombredanne merged commit 4165702 into spdx:master Jul 28, 2021
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.

None yet

2 participants