code-pushup / cli

A CLI to run all kinds of code quality measurements to align your team with company goals
https://code-pushup.dev
MIT License
196 stars 11 forks source link

terminal options as object (`--persist.format`) overwrite the full property of `nx-plugin:autorun` executor options in `package.json` #753

Open BioPhoton opened 1 month ago

BioPhoton commented 1 month ago

What happened?

When I have a executor option like this:

// target config i project.json

{
    "code-pushup": {
        "executor": "@code-pushup/nx-plugin:autorun",
        "options": {
            "persist" : {
              "outputDir": "my-dir",
              "filename": "my-report" 
            }
      }
    },
}

and a terminal argument that touches that object like this:

nx code-pushup --persist.filename=terminal-report

The logged options executor now runs with are:

{
  "persist" : {
    "filename": "terminal-report" 
  }
}

What would you expect to happen?

I expect the executor options after passinf the --persist.filename=terminal-report to keep the outputDir:

{
  "persist" : {
    "outputDir": "my-dir",
    "filename": "terminal-report" 
  }
}

What steps did you take?

place configuration in shape of an object into schema and terminal args

Code PushUp package version

No response

What operation system are you on?

MacOS

Node version

No response

Related

Potentially related: https://github.com/nrwl/nx/issues/26611

matejchalk commented 1 month ago

I have the opposite expectations :laughing: I wouldn't expect --persist.filename to effectively reset other persist settings.

I think we should have consistent handling of nested property overrides, there shouldn't be one rule for persist and another rule for upload, for example. A reasonable use-case is to define most of the required upload options (server, organization, project) in the config file, but set the secret API key on the command-line using --upload.apiKey=${{ secrets.CP_API_KEY }}. This can only work if we combine the config and CLI arguments.

vmasek commented 1 month ago

Adjusted the expected options

BioPhoton commented 1 month ago

There was a typo that cause confusion. Now it is fixed and we agreed on it.