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

Add commit-lockfile-summary to flake nixConfig whitelist #8220

Merged
merged 2 commits into from Apr 18, 2023

Conversation

accelbread
Copy link
Contributor

Motivation

Git repos with flakes may have their own commit title conventions. In those cases, when using nix flake update --commit-lock-file, the user needs the commit-lockfile-summary nix option set to an appropriate commit title.

Using --commit-lock-file rather than manually creating a commit is still desirable as it sets an useful commit description. In order to set this setting, a flake can set in in the flakes nixConfig attribute. However, since this is not a whitelisted option, it will come up with a prompt or warning for most nix commands.

Since this option only sets the commit title for --commit-lock-file, and setting it in a flake would only affect commits for that repo, it is safe to use. Therefore, it should be whitelisted to make setting it painless for flake authors.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command labels Apr 14, 2023
@accelbread accelbread changed the title Whitelist commit-lockfile-summary in flake nixConfig Add commit-lockfile-summary to flake nixConfig whitelist Apr 14, 2023
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Seems like a sensible setting to always accept.
git commit -m does not seem unsafe.

Off-topic: the name is a bit weird, commit-lockfile-summary. Don't we always commit the lockfile summary?

@accelbread
Copy link
Contributor Author

Usually, the lockfile is only staged not committed; --commit-lock-file additionally commits it, and commit-lockfile-summary is the summary for --commit-lock-file so the naming makes sense to me

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

I think this is worth a drive-by improvement to presentation.

src/nix/flake.md Outdated
Comment on lines 384 to 387
whitelisted options (currently `bash-prompt`, `bash-prompt-prefix`,
`bash-prompt-suffix`, and `flake-registry`) are allowed to be set without
confirmation so long as `accept-flake-config` is not set in the global
configuration.
`bash-prompt-suffix`, `flake-registry`, and `commit-lockfile-summary`)
are allowed to be set without confirmation so long as `accept-flake-config`
is not set in the global configuration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Given this is a substantial list now, it makes sense to present it as such. It also reveals that the sentence is a bit clumsy and should be reworded:

Suggested change
whitelisted options (currently `bash-prompt`, `bash-prompt-prefix`,
`bash-prompt-suffix`, and `flake-registry`) are allowed to be set without
confirmation so long as `accept-flake-config` is not set in the global
configuration.
`bash-prompt-suffix`, `flake-registry`, and `commit-lockfile-summary`)
are allowed to be set without confirmation so long as `accept-flake-config`
is not set in the global configuration.
set of options is allowed to be set without confirmation so long as [`accept-flake-config`](@docroot@/command-ref/conf-file.md#conf-accept-flake-config) is not enabled in the global configuration:
- [`bash-prompt`](@docroot@/command-ref/conf-file.md##conf-bash-prompt)
- [`bash-prompt-prefix`](@docroot@/command-ref/conf-file.md#conf-bash-prompt-prefix)
- [`bash-prompt-suffix`](@docroot@/command-ref/conf-file.md#conf-bash-prompt-suffix)
- [`flake-registry`](@docroot@/command-ref/conf-file.md#conf-flake-registry)
- [`commit-lockfile-summary`](@docroot@/command-ref/conf-file.md#commit-lockfile-summary)

This should make those options much more discoverable.

Copy link
Member

Choose a reason for hiding this comment

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

Can't commit in GitHub UI: "Applying suggestions on deleted lines is currently not supported."
Fixed two types and committed by hand.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-47/27387/1

@roberth roberth enabled auto-merge April 18, 2023 14:08
@roberth roberth merged commit 28d7ffd into NixOS:master Apr 18, 2023
8 checks passed
@accelbread accelbread deleted the whitelist-commit-lockfile-summary branch April 20, 2023 05:38
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/this-month-in-nix-docs-2-april-2023/27899/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation new-cli Relating to the "nix" command
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants