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

Make number of classes per generated file configurable #153

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

usiems
Copy link
Contributor

@usiems usiems commented Nov 17, 2023

This introduces the --max-classes-per-file command line argument. The default for this is 30, as before.

The motivation for this is that comparing the output for different versions (of Qt or the generator) gets easier when everything is one file, so that added or removed classes don't change the output of subsequent files. For this purpose one can use --max-classes-per-file=10000.

@@ -62,6 +62,7 @@ class GeneratorSetQtScript : public GeneratorSet

private:
MetaQtScriptBuilder builder;
int maxClassesPerFile{30};
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should use a large number as default instead to make changes by default better comparable. I assume that the restriction had to do with limitations / performance issues with earlier compilers and IDEs, and I doubt that this is still needed nowadays. Would like to hear other opinions...

@usiems usiems force-pushed the classes_per_file_argument branch from 07850a8 to c6be9bf Compare November 17, 2023 19:19
@mrbean-bremen
Copy link
Contributor

I'm going to merge this as it is needed for the other PR. We can always change the default later.

@mrbean-bremen mrbean-bremen merged commit 099eb92 into master Nov 17, 2023
11 checks passed
@mrbean-bremen mrbean-bremen deleted the classes_per_file_argument branch November 17, 2023 19:51
@mrbean-bremen
Copy link
Contributor

Actually it is the other PR which is needed...

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