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

Transient interface for Nix flake commands #140

Merged
merged 20 commits into from
Nov 9, 2021

Conversation

akirak
Copy link
Contributor

@akirak akirak commented Nov 5, 2021

This is an attempt to implement #104.

I don't know if you will like this, but it has helped me with building flakes, so I will propose a draft PR.

@matthewbauer
Copy link
Member

Looks good to me so far!

Show flake:* names as well for convenience in completing registry
entries.
- Add docstrings and autoloads.
- Follow the naming conventions of readers.
- Avoid use of colons in symbol names
According to the guideline, *arguments* refer to required arguments,
which is either a flake ref or installable for most commands.
*Options* refer to both boolean flags and other optional arguments
starting with hyphen. It is better to stick with the terminology.

Also, options are appended to the command line after the required
arguments, so follow the rule as well.

For details, see https://nixos.org/manual/nix/stable/contributing/cli-guideline.html
@akirak
Copy link
Contributor Author

akirak commented Nov 9, 2021

I squashed commits and added transient to the dependency list in flake.nix.

CI fails, but it is probably due to an issue with install-nix-action, which may be soon fixed by cachix/install-nix-action#105. Please wait for a while.

@akirak akirak marked this pull request as ready for review November 9, 2021 06:48
@domenkozar
Copy link
Member

You need to bump to 14.v1 to fix the CI error

Based on the comment by Domen Kožar (@domenkozar) at
NixOS#140 (comment)

Co-authored-by: Domen Kožar <[email protected]>
@akirak
Copy link
Contributor Author

akirak commented Nov 9, 2021

@domenkozar Thank you. It's working!

@matthewbauer matthewbauer self-requested a review November 9, 2021 16:41
matthewbauer pushed a commit that referenced this pull request Nov 9, 2021
Based on the comment by Domen Kožar (@domenkozar) at
#140 (comment)

Co-authored-by: Domen Kožar <[email protected]>
(cherry picked from commit 2a24b40)
@matthewbauer matthewbauer merged commit e7bf2e4 into NixOS:master Nov 9, 2021
@akirak
Copy link
Contributor Author

akirak commented Nov 9, 2021

Thank you so much for merging this PR!

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.

3 participants