-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
doc: primops: add more info for foldl #9254
Conversation
PS: it took me quite some time to figure out where I have to add this improvement. The edit button on the documentation site is misleading. |
Btw the |
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.
@fricklerhandwerk is the doc guru. Looks good?
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.
Great catch, thanks a lot.
We can solve this by either adding more info (as I did) or by changing the example from
foldl' (x: y: x + y) 0 [1 2 3]
tofoldl' (accumulator: next: accumulator + next) 0 [1 2 3]
. Which do you prefer?
I prefer both. But maybe call it item
or element
instead, because it's the current one being processed, not the next. Functional programming is still a challenge for a large part of our growing audience, so let's try to keep the potential for confusion low.
Please follow the docs contribution guide, in particular the one sentence per line rule.
The edit button is useless for many pages indeed, because they are generated, as you've found. It's a low-effort stop-gap. I'd appreciate and swiftly review PRs to update the contribution guide with more help to find the right things. Another, possibly slightly more scalable (but possibly counterproductive) option would be to trick the button into working by adding symlinks in the locations it expects to where the code actually is, and possibly massaging edit-url-template
appropriately.
Side note to @nixos/nix-team: on top of that we may want to follow @infinisil's lead moving contributor-facing documentation in Nixpkgs into the source tree and out of the manual altogether. That would simplify a few things, such as linking to files in their most recent version. (I know, links to files should not be needed with things like doxygen, but reality is messy.) Also, any released manual will be outdated with respect to the code, and we'd anyway want to get (potential) contributors closer to where the action is - on GitHub.
From the existing doc it is not obvious whether the first or the second argument is the accumulator. This is however relevant to know, as for certain scenarios, this might change the behavior.
@fricklerhandwerk I've updated the MR with your suggestions. What do you think about the new version? |
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.
Looks good. I'd usually be picky about formatting according to the style guide, but this can also be done later. Thanks a lot!
Ah this is where I saw you mention this! I opened a separate issue for this :) #9381 |
Motivation
Improving documentation of
builtins.foldl
.From the existing doc it is not obvious whether the first or the second argument of
builtins.foldl'
is the accumulator. This is however relevant to know, as for certain scenarios, this might change the behavior.We can solve this by either adding more info (as I did) or by changing the example from
foldl' (x: y: x + y) 0 [1 2 3]
tofoldl' (accumulator: next: accumulator + next) 0 [1 2 3]
. Which do you prefer?Context
For reference:
Priorities
Not very important.