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

Make option docs evaluate #25

Merged
merged 3 commits into from
Oct 21, 2024
Merged

Make option docs evaluate #25

merged 3 commits into from
Oct 21, 2024

Conversation

roberth
Copy link
Contributor

@roberth roberth commented Sep 28, 2024

Required for hercules-ci/flake.parts-website#1070 to evaluate

Some defaults should probably be normal definitions or lib.mkDefault
definitions in the config section. They don't all make sense to be
documented defaults.

However, I've solved it with some less than optimal descriptions in
order not to move code around. Maybe I should have just moved it?

A few options are still missing descriptions, as shown by the build log:

nix build github:srid/flake.parts-website/rust-flake#checks.x86_64-linux.linkcheck --override-input rust-flake . --no-show-trace -vL
[...]
options.json> error: option perSystem.rust-project.crane-lib has no description
options.json> error: option perSystem.rust-project.crates.<name>.cargoToml has no description
options.json> error: option perSystem.rust-project.crates.<name>.crane.outputs.checks has no description
options.json> error: option perSystem.rust-project.crates.<name>.crane.outputs.packages has no description
options.json> error: option perSystem.rust-project.crates.<name>.path has no description

This PR is meant to get you across the initial bump of making it eval, so we can get the docs to build. Could you review and complete it? Feel free to push to my fork's branch.

Some general recommendations:

  • Some definitions where a documented default isn't needed could be lib.mkDefault in config instead
  • Please add a command like the above to CI (replacing srid's branch by upstream when merged)
  • Always add a defaultText if the default isn't a constant literal / references any variable, option or function

Some defaults should probably be normal definitions or `lib.mkDefault`
definitions in the config section. They don't all make sense to be
documented defaults.

However, I've solved it with some less than optimal descriptions in
order not to move code around. Maybe I should have just moved it?
@srid
Copy link
Member

srid commented Oct 21, 2024

@roberth Appreciate this PR; sorry I had let it slip. Will merge soonish.

@srid srid merged commit a517599 into juspay:main Oct 21, 2024
3 checks passed
@roberth
Copy link
Contributor Author

roberth commented Oct 21, 2024

Don't worry about it :)

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.

2 participants