Skip to content

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Feb 12, 2024

Motivation

Context

Blocked on #9994, tests will fail until this

A little progress on #10311

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@thufschmitt
Copy link
Member

Discussed during the Nix maintainers meeting on 2024-02-26.
Idea approved, but :

  • Should be mentioned in the release notes
  • Should be reflected in the json guidelines policy
Details
  • @edolstra: Potentially breaking change, although probably not a big one

    • Also only happens in the new command-line
  • @thufschmitt: Do we care about having these null fields?

    • @roberth: Have a slightly different semantics, potentially interesting to distinguish
  • Idea approved

  • Should be mentioned in the release notes

  • Should be reflected in the json guidelines policy

@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-02-28-nix-team-meeting-129/40499/1

@Ericson2314 Ericson2314 marked this pull request as ready for review April 5, 2024 17:27
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Apr 5, 2024
@Ericson2314 Ericson2314 added this to the nix-command stabilisation milestone Apr 5, 2024
@Ericson2314 Ericson2314 marked this pull request as draft May 30, 2024 14:42
@Ericson2314 Ericson2314 marked this pull request as ready for review May 31, 2024 21:46
@github-actions github-actions bot added documentation contributor-experience Developer experience for Nix contributors labels May 31, 2024
@Ericson2314 Ericson2314 force-pushed the json-empty-sigs branch 3 times, most recently from 335184d to f2195ef Compare May 31, 2024 21:56
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.

Have only reviewed docs so far.

Copy link
Member

Choose a reason for hiding this comment

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

Thoughts: we should recommend that consumers ignore any fields they don't know.
I also like the GraphQL-like idea of specifying which attrs to expect, so that the consumer can safely use the whole value without anything unexpected occurring in it.
However, this seems less relevant (and unlikely to see much adoption) in the CLI; rather applies more to a fetchTree meta return attr (#8940 (comment)), where use of the whole object is actually relevant for reproducibility. Or as a solution for in-sandbox path info JSONs that isn't quite as capable as versioning, but does prevent over-fetching.

@Ericson2314 Ericson2314 force-pushed the json-empty-sigs branch 2 times, most recently from 387a3a5 to 1d6c50a Compare June 1, 2024 16:08
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.

Nice!

Co-authored-by: Robert Hensing <[email protected]>
Co-authored-by: Robert Hensing <[email protected]>
@Ericson2314 Ericson2314 enabled auto-merge June 3, 2024 12:28
@Ericson2314 Ericson2314 merged commit c6add88 into master Jun 3, 2024
@Ericson2314 Ericson2314 deleted the json-empty-sigs branch June 3, 2024 12:58
@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-06-03-nix-team-meeting-minutes-149/46582/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor-experience Developer experience for Nix contributors documentation with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants