Skip to content

[SUGGESTION] Refactorings for to_cpp1.h file #1047

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

Closed
wants to merge 7 commits into from

Conversation

dutkalex
Copy link
Contributor

@dutkalex dutkalex commented Mar 31, 2024

This PR contains a handful of small helper extractions to simplify the to_cpp1.h file, which reduces its length by about 1000 lines. Indeed, this file is very long and I believe that there are a lot of low hanging fruits like these ones... What do you think about that?

@dutkalex
Copy link
Contributor Author

dutkalex commented Mar 31, 2024

Behavior should be exactly the same, to the notable exception of the value of source_loaded in the case of a wrong file extension. Indeed, with the current implementation source_loaded == true in that case although file loading is bypassed, which I believe is a bug...

@hsutter
Copy link
Owner

hsutter commented May 8, 2024

Thanks for your pull request! It looks like this may be your first contribution to cppfront. I need to send you the Contributor License Agreement (CLA) but I don't have your email address -- please email me at first dot last at gmail. Once it's signed I can look at your pull request. Thanks again for your contribution.

@dutkalex
Copy link
Contributor Author

dutkalex commented May 8, 2024

@hsutter email sent :)

dutkalex added 3 commits May 11, 2024 15:41
Signed-off-by: Alex Dutka <[email protected]>
Signed-off-by: Alex Dutka <[email protected]>
@dutkalex
Copy link
Contributor Author

It seems like something went wrong here, but I won't try to fix it without knowing first if this is something you are interested in...

@dutkalex
Copy link
Contributor Author

Seems like the problem has been fixed...

@hsutter
Copy link
Owner

hsutter commented Sep 25, 2024

Thanks! Sorry for the lag on this...

Organization note: I want to keep the current file structure and not break to_cpp1.h into separate files at this point, sorry.

I appreciate this but I don't have the cycles to review this now per the priorities in #1287, so for now I'll close this PR. (Sorry!)

If there is a subset of this where you see direct duplication in to_cpp1.h that can be eliminated in a much-lower-impact PR that wouldn't require much review, I'd be open to that as a new PR. Otherwise I realize there's a bit of duplication but the code is working reasonably well and I need to prioritize new features and fixing blocking bugs over code cleanup at this stage. Thanks for understanding.

@hsutter hsutter closed this Sep 25, 2024
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.

2 participants