-
-
Notifications
You must be signed in to change notification settings - Fork 312
Improvement of Note on Fetchers #1191
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
base: master
Are you sure you want to change the base?
Conversation
…ers and nixkgs fetchers are not the same
hsjobeki
left a comment
There was a problem hiding this 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 a lgtm improvement.
However we should be more concrete.
- "more optimized evaluation" -> optimized how? For what use case? This is too vague.
- "differ in some aspects" -> which aspects? Same problem.
This should be more concrete.
Suggestion: highlight the most important difference. Paused eval i assume.
For the rest place a reference to the manual.
|
Thanks for the feedback @hsjobeki. I have chosen to highlight the additional hash argument in the nixpkgs fetchers. That will also make them recognizable and is probably the most apparent difference for anyone not too deep into the details. Feel free to give me additional feedback if you think something could be better :) |
Co-authored-by: Johannes Kirschbauer <hsjobeki+github@gmail.com>
|
Your suggestions read much better @hsjobeki . I will take the tips and try to write better next time. |
Co-authored-by: Valentin Gagarin <valentin@gagarin.work>
| The Nixpkgs manual on [Fetchers][nixpkgs-fetchers] lists numerous additional library functions to fetch files over the network. | ||
| Nixpkgs provides its own fetcher functions to reduce the time needed for evaluating Nixpkgs. The Nixpkgs fetchers are not directly interchangeable with builtin fetchers; for example, the Nixpkgs fetchers require a hash argument. | ||
|
|
||
| ::::{note} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you unify this into only one :::{note} block?
| ::::{note} |
Also the following three sentences should be trimmed down to only contain essential and required information.
Instead of repeating one of them in a nested :::note
- Nixpkgs provides its own fetcher functions to reduce the time needed for evaluating Nixpkgs. -> Nixpkgs provides its own fetcher functions to reduce the time needed for evaluation.
- The Nixpkgs fetchers are not directly interchangeable with builtin fetchers; for example, the Nixpkgs fetchers require a hash argument. ->
They are not directly interchangeable with builtins fetchers - Nixpkgs provides its own fetcher functions with important differences from the builtin fetchers: ->
Important differences:
for example, the Nixpkgs fetchers require a hash argument. can be left out, because you explicitly state that later in the bullet list.
I wanted to make this change to emphasize the difference between the builtin fetchers and the nixpkgs fetchers.