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

Stuctured command stability #7611

Merged

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jan 17, 2023

Prior to this, there was an ad-hoc whitelist in main.cc. Now, every command states its stability.

In a future PR, we will adjust the manual to take advantage of this new
information in the JSON. It will be easier to do that once we have some experimental feature docs to link too; see #5930 and #7798.)

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Jan 17, 2023

The diff is quite large and it's not clear at a glance what the change actually is about. Can you elaborate in the PR description what exactly changed in terms of user-visible behavior?

This seems more fitting:

Gate nix --help sections based on which experimental features are enabled

PS: "Clean up" should be a forbidden phrase in commit messages.

@dpulls
Copy link

dpulls bot commented Jan 17, 2023

🎉 All dependencies have been resolved !

@Ericson2314
Copy link
Member Author

The diff is large because of the other PRs this depends on.

@Ericson2314 Ericson2314 marked this pull request as draft January 17, 2023 16:16
@Ericson2314
Copy link
Member Author

Draft because https://github.com/apps/dpulls is not cutting it, and the chained diffs are confusing.

@Ericson2314 Ericson2314 mentioned this pull request Feb 13, 2023
86 tasks
@github-actions github-actions bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Apr 2, 2023
@Ericson2314 Ericson2314 force-pushed the structured-command-stabilization branch from b16d91f to 913888f Compare April 2, 2023 23:35
@github-actions github-actions bot removed the with-tests Issues related to testing. PRs with tests have some priority label Apr 2, 2023
@Ericson2314 Ericson2314 marked this pull request as ready for review April 2, 2023 23:35
@Ericson2314 Ericson2314 marked this pull request as draft April 3, 2023 11:54
@fricklerhandwerk
Copy link
Contributor

Discussed in the Nix team meeting 2023-04-03:

  • @edolstra: not in favor of dynamically generated documentation
    • it should be just markdown files, and the raw string technique is just a hack
    • the flag is good, but should rather use it to mark experimental features rather than hiding it
    • we can also put the information into the prose
    • @fricklerhandwerk: agreed, to gather experience we have to expose the functionality in documentation
  • agreement:
    • merge the infrastructure first
    • deal with rendering experimental stuff later
    • --help language on installable and flakes should be static and can be an independent PR

@Ericson2314 Ericson2314 force-pushed the structured-command-stabilization branch from 913888f to e97c91f Compare April 3, 2023 14:45
@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority and removed documentation labels Apr 3, 2023
@Ericson2314 Ericson2314 changed the title Clean up nix --help Stuctured command stability Apr 3, 2023
@Ericson2314 Ericson2314 marked this pull request as ready for review April 3, 2023 15:08
@Ericson2314
Copy link
Member Author

This is now just the infrastructure, as discussed.

@Ericson2314 Ericson2314 force-pushed the structured-command-stabilization branch from e97c91f to 1a71eca Compare April 3, 2023 15:10
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.

Just the nit, otherwise lgtm!

src/libutil/args.cc Outdated Show resolved Hide resolved
@Ericson2314 Ericson2314 force-pushed the structured-command-stabilization branch from 1a71eca to 3c059ce Compare April 3, 2023 15:48
Prior to this, there was an ad-hoc whitelist in `main.cc`. Now, every
command states its stability.

In a future PR, we will adjust the manual to take advantage of this new
information in the JSON.
(It will be easier to do that once we have some experimental feature
docs to link too; see NixOS#5930 and NixOS#7798.)
@Ericson2314 Ericson2314 force-pushed the structured-command-stabilization branch from 3c059ce to 4a0b893 Compare April 3, 2023 15:48
@Ericson2314 Ericson2314 enabled auto-merge April 3, 2023 15:48
@Ericson2314 Ericson2314 merged commit acc3314 into NixOS:master Apr 3, 2023
@Ericson2314 Ericson2314 deleted the structured-command-stabilization branch April 3, 2023 16:22
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-04-03-nix-team-meeting-minutes-46/27008/1

@thufschmitt thufschmitt added this to the CLI Stabilisation milestone Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants