Skip to content

Add tool CGConvert to convert between file format versions#39

Merged
pearzt merged 3 commits intotudasc:develfrom
pearzt:feat/cgconvert
May 27, 2025
Merged

Add tool CGConvert to convert between file format versions#39
pearzt merged 3 commits intotudasc:develfrom
pearzt:feat/cgconvert

Conversation

@pearzt
Copy link
Member

@pearzt pearzt commented May 22, 2025

This is pretty much just what the title says. I was looking for such a tool to generate test cases with file format version 3 to pymetacg, didn't find one, so I cobbled this together.

@pearzt pearzt self-assigned this May 22, 2025
Copy link
Member

@jplehr jplehr left a comment

Choose a reason for hiding this comment

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

Initial review.

The cxxopts comment is a wish-thing. I do understand that it may look like overkill for such a simple tool. Please look at it though and make a reasonable decision.

Copy link
Member

@sebastiankreutzer sebastiankreutzer left a comment

Choose a reason for hiding this comment

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

LGTM. There is always the question about tests, but in this case I think the conversion is handled by the graph lib unit tests.
IMO, cxxopts is not necessary here due to the simplicity (we also don't use it for cgmerge(2)).

@jplehr
Copy link
Member

jplehr commented May 24, 2025

IMO, cxxopts is not necessary here due to the simplicity (we also don't use it for cgmerge(2)).

Yeah, I can understand that point of view. I mostly think that having a uniform way of doing things is beneficial. So, IMHO, we should strive towards a more uniform way, i.e., use cxxopts for everything instead of having this arbitrary "this is simple enough" metric.

Not a blocker, and not the most important thing. Still my point of view.

Copy link
Member

@TimHeldmann TimHeldmann left a comment

Choose a reason for hiding this comment

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

Very useful, thank you!
Looks good to me, just two questions.

@pearzt pearzt merged commit dd0d218 into tudasc:devel May 27, 2025
3 checks passed
@pearzt pearzt deleted the feat/cgconvert branch August 8, 2025 11:06
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.

4 participants