Skip to content

feat(ci): skip persist.format args if defaults already configured #1042

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

Merged
merged 2 commits into from
Jul 31, 2025

Conversation

matejchalk
Copy link
Collaborator

Final piece from larger effort to avoid CLI arguments which reduce chances of Nx cache hits in default setup. See also:

The CLI arguments --persist.format=json --persist.format=md were being set in case user-defined config omitted report formats which @code-pushup/ci relies on. After this refactor, the configured formats are first checked in print-config output, and then only set if some format is missing - in monorepo mode with bulk collect, every project must have default formats configured in order to skip the CLI arguments.

TLDR: --persist.format args are now skipped unless user has some unusual config.

@beaussan
Copy link
Contributor

👋 If I get this right, for each project in the monorepo code pushup ci would run, in sequence, a config command before. On a monorepo of a 100 project, given a median time of 2s, that's 200s before collect starts.

Could we instead provide an option to ci entrypoint like skipFormatsParameter that would print a warning at the beginning (something like "code pushup expect the format to be set to md and json. If not, ci mode could fail") and not pass the format args?

@matejchalk
Copy link
Collaborator Author

👋 If I get this right, for each project in the monorepo code pushup ci would run, in sequence, a config command before.

Yes, but other than before replacing after, this PR doesn't change any of that.

We're running the print-config command for each project anyway, because it's the only way we can reliably figure out where each project's report files were created. We tried running it in parallel (Promise.all) originally, but Nx couldn't handle that, so we switched to sequential execution in #992 to prevent errors.

Since we were already reading each project's persist config, I decided it was simplest to reuse that data to check the configured formats. For bulk collect for current state in monorepo mode, that required me to swap the order, for other collect executions (including bulk collect for previous state) print-config was being executed before collect anyway.

On a monorepo of a 100 project, given a median time of 2s, that's 200s before collect starts.

Correct me if I'm wrong, but I'm assuming it's just the extra processing time (which isn't new) that's the problem? Or does it make a difference if it's before or after collect?

Could we instead provide an option to ci entrypoint like skipFormatsParameter that would print a warning at the beginning (something like "code pushup expect the format to be set to md and json. If not, ci mode could fail") and not pass the format args?

As mentioned above, I think such a change is beyond the scope of this PR, because the print-config per project is being executed anyway. For this reason, it would need a more generalized approach, because @code-pushup/ci needs to have some reliable way of knowing that all the files will be created and where.

Your suggestion of having the user set a "all my projects are configured in this way, trust me and don't waste time checking it, please" option could apply to persist configs in general, for example.

Another solution I can think of is to have some alternative way for @code-pushup/ci to override persist config other than CLI args, e.g. some special environment variable - pro is no user configuration needed, con is it'd introduce more configuration complexity (env vars > CLI args > config file).

Either way, the outputDir at least can't be a static value, so we'd need to establish some pattern syntax (e.g. Nx-style {projectName} interpolation) so it can work for bulk collect (value can't differ per project if it's one command for all).

We could also revisit running print-configs in parallel, this time via nx run-many. Again, we'd some some interpolated pattern to know where to read the config.json from, currently each print-config used a --output CLI argument to set this path.

@matejchalk matejchalk merged commit 54cb3f8 into main Jul 31, 2025
15 of 16 checks passed
@matejchalk matejchalk deleted the avoid-ci-format-args branch July 31, 2025 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants