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

Merge how-to section on S3 buckets into S3 store docs #7972

Merged
merged 8 commits into from
Oct 23, 2023

Conversation

fricklerhandwerk
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk commented Mar 5, 2023

Motivation

these descriptions are not reference documentation, therefore should
not be part of the manual, and instead be moved to a curated collection of how-to guides.

Rather than having a misc tutorial page in the grab-bag "package management" section, this information should just be part of the S3 store docs.

Blocked by NixOS/nix.dev#488 no longer, now that this is moving not deleting.

Context

a first step towards #7769

  • move remaining reference documentation to the appropriate location.
  • remove the Amazon-side samples, this should not be maintained here.

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
  • 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.

fricklerhandwerk added a commit to fricklerhandwerk/nixos-homepage that referenced this pull request Mar 5, 2023
page removed in NixOS/nix#7972

Requesting a pull to NixOS:master from fricklerhandwerk:manual-amazon-redirect

Write a message for this pull request. The first block
of text is the title and the rest is the description.
fricklerhandwerk added a commit to fricklerhandwerk/nixos-homepage that referenced this pull request Mar 5, 2023
@fricklerhandwerk fricklerhandwerk force-pushed the remove-package-management branch 2 times, most recently from 98bc739 to 7ba8211 Compare March 5, 2023 03:30
@edolstra
Copy link
Member

edolstra commented Mar 6, 2023

I'm not sure documentation on the required S3 bucket policy isn't reference material. At the very least, it's important information that should be provided somewhere; we shouldn't just delete it.

@fricklerhandwerk
Copy link
Contributor Author

I'm arguing this is on Amazon to provide, unless there is something really special Nix needs here (which I don't currently see, but I may be completely wrong). We could link to it, but then we'd have to maintain it here. I in principle agree on providing as much information as possible in one place to avoid people having to jump sites, and also on not removing pre-existing information, but we have to balance that with maintenance effort. There's no point in ending up having outdated documentation on things that are beyond our domain of responsibility.

@roberth
Copy link
Member

roberth commented Mar 12, 2023

There's no point in ending up having outdated documentation on things that are beyond our domain of responsibility.

A point is that contributors can't update the documentation if there isn't any documentation to start with.

I don't perceive a particularly high maintenance cost here. The only real cost I see would be to periodically verify that the instructions are correct. I don't think that's our responsibility for instructions like these. We should be able to rely on drive-by contributions for this. Also Amazon doesn't have quite the hunger for breaking changes that some of us seem to have; quite the opposite.

@symphorien
Copy link
Member

I just used these instructions, they are useful and most importantly have no replacement. Please don't remove them.

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.

I would like to close this. Maybe some minor edits can be salvaged?


At least one of the following conditions must be met for Nix to use
a substituter:

- the substituter is in the [`trusted-substituters`](#conf-trusted-substituters) list
- the user calling Nix is in the [`trusted-users`](#conf-trusted-users) list

In addition, each store path should be trusted as described
in [`trusted-public-keys`](#conf-trusted-public-keys)
In addition, each store path should be trusted as described in [`trusted-public-keys`](#conf-trusted-public-keys).
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a good place for substituter-specific information. It'd be better to refer to sections about each specific substituter type. This isn't scalable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose this is addressed by #8084

@fricklerhandwerk
Copy link
Contributor Author

fricklerhandwerk commented Mar 22, 2023

@symphorien @roberth I added a blocker on NixOS/nix.dev#488 to make sure the content is not lost.

PS: I don't want to destroy information, the point is to make it easier to find. See it as a clumsy attempt at mimicking Eelco's technique of removing things to see if there is an outcry in order to gauge interest.

@aakropotkin
Copy link
Contributor

You have like 5 PRs deleting useful documentation without publishing it elsewhere.

Your expectation that people will notice it's gone and request it to be added back makes absolutely no sense.

You're making these features and information undiscoverable except by people who already know the information.

Cut it out.

If you first create a more appropriate location to put these articles, publish them, and make them discoverable from this manual - then it might be sensible to start migrating articles out. Until then these PRs are just making nix harder to use and understand.

@edolstra
Copy link
Member

Now that #8084 is merged, the S3 documentation could be moved to src/libstore/s3-binary-cache-store.md. It already removed the part of doc/manual/src/package-management/s3-substituter.md about the settings, since that's now auto-generated.

@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-04-14-nix-team-meeting-minutes-48/27358/1

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 14, 2023

Yeah let's redo using the s3 store docs. In general, it is very good when auto docs provide structure allowing us to make fewer decisions!

@github-actions github-actions bot added new-cli Relating to the "nix" command and removed documentation labels Jun 15, 2023
these descriptions are not reference documentation and therefore should
not be part of the manual.

move remaining reference documentation to the appropriate location.

remove the Amazon-side samples, this should not be maintained here.
@github-actions github-actions bot removed the new-cli Relating to the "nix" command label Oct 9, 2023
This is basically the information just removed, but now in a more
structured location (The S3 store's docs, alongside the docs for the
other stores). The wording is also slightly changed to be more in a
reference rather than tutorial style.
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Oct 9, 2023
@Ericson2314 Ericson2314 changed the title remove how-to section on S3 buckets Merge how-to section on S3 buckets into S3 store docs Oct 9, 2023
@Ericson2314
Copy link
Member

OK I added a commit to move this information to that page, and slightly change it to be in a more reference style. I hope that makes this PR is now longer controversial ;).

@Ericson2314 Ericson2314 dismissed roberth’s stale review October 9, 2023 17:45

Now that we are no deleting documenation

src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
src/libstore/s3-binary-cache-store.md Outdated Show resolved Hide resolved
@Ericson2314 Ericson2314 merged commit cd680bd into NixOS:master Oct 23, 2023
8 checks passed
@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-documentation-team-meeting-notes-88/34562/1

tomberek added a commit to NixOS/nixos-homepage that referenced this pull request Nov 8, 2023
blocked by: NixOS/nix#7972

---------

Co-authored-by: tomberek <tomberek@users.noreply.github.com>
@kirillrdy
Copy link
Member

after this PR, there is still an empty section left in the manual

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
store Issues and pull requests concerning the Nix store
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants