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

feat: Support command selection with the config command #22

Merged
merged 2 commits into from
Jan 26, 2025

Conversation

mareksapota
Copy link
Contributor

@mareksapota mareksapota commented Jan 25, 2025

What type of PR is this?

feat: The config command is great for centralizing nwa configuration in a project. It is weird though that the command being executed is part of the configuration. My expectation for the config command would be to use pre-set paths, template, skip rules, etc. so one could run add/update manually and check in CI using the same configuration. Since YAML doesn't support importing currently one has to copy/paste configuration files for each command being executed.

This change adds -c and --command switches to the check command that can overwrite the default command. The priority order, low to high, is the default: add, the command line flag and what is configured in the config file with the highest priority.

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.

(Optional) More detail description for this PR.

Test plan:

$ buildah build -t ghcr.io/b1nary-gr0up/nwa:main .
$ cat <<EOF > .nwa-config.yml
nwa:
  path:
    - "**/*.go"
  license: apache
  holder: "BINARY Members"
  year: 2023
EOF
$ # command line command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml -c check
2025/01/25 12:09:51 WARN file does not have a matched header path=internal/header.go
[NWA SUMMARY] scanned=11 matched=10 mismatched=1 skipped=0 failed=0
$ # default command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml
2025/01/25 12:10:34 WARN file already has a header path=cmd/add.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/check.go
2025/01/25 12:10:34 WARN file already has a header path=internal/task.go
2025/01/25 12:10:34 WARN file already has a header path=internal/operation.go
2025/01/25 12:10:34 WARN file already has a header path=internal/tmpl.go
2025/01/25 12:10:34 WARN file already has a header path=main.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/remove.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/config.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/root.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/update.go
2025/01/25 12:10:34 WARN file already has a header path=internal/header.go
[NWA SUMMARY] scanned=11 modified=0 skipped=0 failed=0
$ cat <<EOF > .nwa-config.yml
nwa:
  path:
    - "**/*.go"
  license: apache
  holder: "BINARY Members"
  year: 2023
  cmd: "check"
EOF
$ # config file command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml
2025/01/25 12:12:21 WARN file does not have a matched header path=internal/header.go
[NWA SUMMARY] scanned=11 matched=10 mismatched=1 skipped=0 failed=0

Which issue(s) this PR fixes:

Fixes #21

Summary: The `config` command is great for centralizing nwa
configuration in a project.  It is weird though that the command being
executed is part of the configuration.  My expectation for the `config`
command would be to use pre-set paths, template, skip rules, etc. so one
could run `add`/`update` manually and `check` in CI using the same
configuration.  Since YAML doesn't support importing currently one has
to copy/paste configuration files for each command being executed.

This change adds `-c` and `--command` switches to the `check` command
that can overwrite the pre-configured command.  The priority order, low
to high, is the default: `add`, what is configured in the config file
and the command line flag with highest priority.

Test plan:
```
$ buildah build -t ghcr.io/b1nary-gr0up/nwa:main .
$ cat <<EOF > .nwa-config.yml
nwa:
  path:
    - "**/*.go"
  license: apache
  holder: "BINARY Members"
  year: 2023
EOF
$ # command line command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml -c check
2025/01/25 12:09:51 WARN file does not have a matched header path=internal/header.go
[NWA SUMMARY] scanned=11 matched=10 mismatched=1 skipped=0 failed=0
$ # default command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml
2025/01/25 12:10:34 WARN file already has a header path=cmd/add.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/check.go
2025/01/25 12:10:34 WARN file already has a header path=internal/task.go
2025/01/25 12:10:34 WARN file already has a header path=internal/operation.go
2025/01/25 12:10:34 WARN file already has a header path=internal/tmpl.go
2025/01/25 12:10:34 WARN file already has a header path=main.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/remove.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/config.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/root.go
2025/01/25 12:10:34 WARN file already has a header path=cmd/update.go
2025/01/25 12:10:34 WARN file already has a header path=internal/header.go
[NWA SUMMARY] scanned=11 modified=0 skipped=0 failed=0
$ cat <<EOF > .nwa-config.yml
nwa:
  path:
    - "**/*.go"
  license: apache
  holder: "BINARY Members"
  year: 2023
  cmd: "check"
EOF
$ # config file command
$ podman run --rm -v './:/src/' ghcr.io/b1nary-gr0up/nwa:main config .nwa-config.yml
2025/01/25 12:12:21 WARN file does not have a matched header path=internal/header.go
[NWA SUMMARY] scanned=11 matched=10 mismatched=1 skipped=0 failed=0
```
@justlorain
Copy link
Member

I think it would be more intuitive to prioritize the command specified by --command with the following priority order:

the command line flag > configured in the config file > default add

However, there's no issue with this PR. I will submit a new commit to address the priority issue.

Thanks for your PR and suggestion, LGTM!

@justlorain justlorain merged commit fef6025 into B1NARY-GR0UP:main Jan 26, 2025
1 check passed
@mareksapota
Copy link
Contributor Author

the command line flag > configured in the config file > default add

I think you're right, that order would be better.

I will submit a new commit to address the priority issue.

Awesome!

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.

Support command selection with the config command
2 participants