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.

Sign up for GitHub

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

Jump to bottom

Document string context #8595

Merged
merged 5 commits into from
May 8, 2024
Merged

Document string context #8595

merged 5 commits into from
May 8, 2024

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Jun 27, 2023 •
edited
Loading

Motivation

Now what we have enough primops, we can document how string contexts work.

Context

Progress towards #7910.

Depends on #9168
Depends on #9216

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@Ericson2314 Ericson2314 requested a review from roberth June 27, 2023 16:18
@github-actions github-actions bot added the tests label Jun 27, 2023
@Ericson2314 Ericson2314 force-pushed the addDrvOutputDependencies branch 2 times, most recently from 1e5d5a7 to 3438db9 Compare June 27, 2023 16:26
@Ericson2314 Ericson2314 mentioned this pull request Jun 27, 2023
Draft
8 tasks
@roberth roberth added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Jul 1, 2023
@Ericson2314 Ericson2314 marked this pull request as ready for review July 9, 2023 18:34
@Ericson2314 Ericson2314 requested a review from edolstra as a code owner July 9, 2023 18:34
roberth
roberth reviewed Jul 14, 2023
View reviewed changes
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.

  • Check that it's .drv
  • Docs need some work

tests/config.nix.in Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jul 14, 2023
@Ericson2314 Ericson2314 force-pushed the addDrvOutputDependencies branch from 38040f8 to 8324fd8 Compare July 17, 2023 13:54
@Ericson2314 Ericson2314 marked this pull request as draft July 17, 2023 13:54
@edolstra
Copy link
Member

edolstra commented Aug 4, 2023

Can you add a description of what the primop does to the PR motivation?

@roberth
Copy link
Member

roberth commented Aug 4, 2023

Can you add a description of what the primop does to the PR motivation?

I've added the three lines at the start.

@Ericson2314
Copy link
Member Author

Triaged in the Nix Team meeting this morning (n.b. I could not make it):

  • Needs to explain what it's about in the PR description
    • @roberth has the context and can do, and now has done so.
  • Also needs to be discussed with the Nixpkgs architecture team to see whether they actually want that

Queued for discussion

@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-08-08-nixpkgs-architecture-team-meeting-42/31454/1

@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-08-04-nix-team-meeting-minutes-77/31487/1

@Ericson2314 Ericson2314 mentioned this pull request Oct 11, 2023
Open
@Ericson2314 Ericson2314 force-pushed the addDrvOutputDependencies branch from 8324fd8 to d934667 Compare October 13, 2023 05:06
@Ericson2314 Ericson2314 marked this pull request as ready for review October 13, 2023 05:06
@thufschmitt thufschmitt self-assigned this Oct 13, 2023
thufschmitt
thufschmitt previously requested changes Oct 16, 2023
View reviewed changes
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
src/libexpr/primops/context.cc Outdated Show resolved Hide resolved
tests/functional/config.nix.in Outdated Show resolved Hide resolved
tests/functional/lang/eval-fail-addDrvOutputDependencies-multi-elem-context.err.exp Outdated Show resolved Hide resolved
doc/manual/src/language/string-context.md Outdated Show resolved Hide resolved
doc/manual/src/language/string-context.md Show resolved Hide resolved
doc/manual/src/language/string-context.md Show resolved Hide resolved
@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-10-23-nix-team-meeting-minutes-97/34561/1

@fricklerhandwerk fricklerhandwerk mentioned this pull request Nov 7, 2023
Draft
fricklerhandwerk
fricklerhandwerk reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ericson2314 do you need help with that? I could quickly take it over if you like, as I have enough context and ideas to finish it. I need a bit more time to ramp up with the your store docs PRs.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Nov 8, 2023 •
edited
Loading

@fricklerhandwerk I think it would be good to do the a deriving path page first, and then you take this over (and some things here can just reference that rather than include redundant material).

@Ericson2314 Ericson2314 mentioned this pull request Nov 16, 2023
Closed
@Ericson2314 Ericson2314 marked this pull request as draft December 1, 2023 14:27
@Ericson2314
Copy link
Member Author

Draft because I wanted deriving paths to be documented first

fricklerhandwerk
fricklerhandwerk reviewed Dec 1, 2023
View reviewed changes
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's pretty useful the way it is. In fact we could already add the glossary entry on deriving paths independently. If you want I can pick up the string contexts part from there and implement the suggestions myself.

doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/language/string-context.md Outdated Show resolved Hide resolved
doc/manual/src/language/string-context.md Outdated Show resolved Hide resolved
@Ericson2314 @roberth
@thufschmitt @fricklerhandwerk @iFreilicht
Document string context
5f05df6 
Now what we have enough primops, we can document how string contexts
work.

Co-authored-by: Robert Hensing <[email protected]>
Co-authored-by: Théophane Hufschmitt <[email protected]>
Co-authored-by: Valentin Gagarin <[email protected]>
Co-authored-by: Felix Uhl <[email protected]>
@Ericson2314 Ericson2314 force-pushed the addDrvOutputDependencies branch from 2615e67 to 5f05df6 Compare December 20, 2023 08:09
@fricklerhandwerk fricklerhandwerk mentioned this pull request Apr 18, 2024
Merged
8 tasks
@fricklerhandwerk fricklerhandwerk marked this pull request as ready for review May 8, 2024 20:25
@fricklerhandwerk fricklerhandwerk enabled auto-merge (squash) May 8, 2024 20:43
@fricklerhandwerk fricklerhandwerk merged commit b560521 into NixOS:master May 8, 2024
9 checks passed
pull bot pushed a commit to auxolotl/nix that referenced this pull request May 9, 2024
@Ericson2314 @roberth
@thufschmitt @fricklerhandwerk @iFreilicht @pull
Document string context (NixOS#8595)
70b1e5b 
* Document string context

Now what we have enough primops, we can document how string contexts
work.

Co-authored-by: Robert Hensing <[email protected]>
Co-authored-by: Théophane Hufschmitt <[email protected]>
Co-authored-by: Valentin Gagarin <[email protected]>
Co-authored-by: Felix Uhl <[email protected]>
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-05-08-nix-team-meeting-minutes-144/45349/1

@fricklerhandwerk fricklerhandwerk mentioned this pull request May 14, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iFreilicht iFreilicht iFreilicht left review comments

@roberth roberth roberth left review comments

@fricklerhandwerk fricklerhandwerk fricklerhandwerk approved these changes

@thufschmitt thufschmitt thufschmitt left review comments

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

Assignees

@fricklerhandwerk fricklerhandwerk

Labels
documentation language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Ericson2314 @edolstra @roberth @nixos-discourse @fricklerhandwerk @thufschmitt @iFreilicht