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

nixos/doc: improve install instructions #175128

Merged
merged 2 commits into from
Oct 26, 2022

Conversation

davidak
Copy link
Member

@davidak davidak commented May 28, 2022

Description of changes

I have tested booting from install drive created with Etcher, USBImager and dd on linux. It would be great if someone can test the other options (macOS and Windows).

I hope to get this into the release.

cc @dasJ

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label May 28, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels May 28, 2022
@davidak davidak force-pushed the doc-graphical-installer branch 2 times, most recently from 5fb71e2 to bc07023 Compare May 28, 2022 17:27
@davidak davidak force-pushed the doc-graphical-installer branch 2 times, most recently from 0bcc72b to fc77271 Compare May 28, 2022 21:54
@davidak davidak marked this pull request as ready for review May 28, 2022 21:57
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
@davidak
Copy link
Member Author

davidak commented May 29, 2022

@samueldr thank you for the review!

@davidak davidak force-pushed the doc-graphical-installer branch 2 times, most recently from 68c1a9f to 02eb42c Compare May 29, 2022 17:33
@dasJ
Copy link
Member

dasJ commented May 29, 2022

I find it unlikely that this will get into the release in time since its a fairly new PR touching a lot of docs and I assume more people want to review it (release schedule is for tomorrow).
That being said, I see no reason we shouldn't backport this at a later point. I don't assume a lot more people will do new installations after the 22.05 release so there's no need to hurry and more time for thorough reviews of this critical piece of docs (one of the first items a new user would touch)

nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
@davidak
Copy link
Member Author

davidak commented May 29, 2022

I don't assume a lot more people will do new installations after the 22.05 release
this critical piece of docs (one of the first items a new user would touch)

i think that is a good reason to merge this big improvement before the release. two topics are not documented at all right now

it might not be perfect, but i think the experience with this change would be better than without it

do you see specific parts that can be controversial? (i will add system requirements in a separate PR)

nixos/doc/manual/installation/obtaining.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing.chapter.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing-usb.section.md Outdated Show resolved Hide resolved
@davidak davidak force-pushed the doc-graphical-installer branch 2 times, most recently from 1b54f33 to 78b8779 Compare May 30, 2022 01:23
@davidak davidak force-pushed the doc-graphical-installer branch 3 times, most recently from 42700a7 to 558577b Compare June 3, 2022 15:29
@davidak
Copy link
Member Author

davidak commented Jun 3, 2022

@Mic92 thanks for your review!

I have incorporated all feedback.

@davidak
Copy link
Member Author

davidak commented Jun 15, 2022

Why is this still not merged?

Everyone is complaining that the documentation is outdated or incomplete and this PR would improve that, but now it's stalled for about 2 weeks. I had plans for further improvements once this is merged, but this contribution experience is very discouraging.

Copy link
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

A few nitpicks.

nixos/doc/manual/installation/installing-usb.section.md Outdated Show resolved Hide resolved
nixos/doc/manual/installation/installing-usb.section.md Outdated Show resolved Hide resolved
and wait (or press <kbd>Enter</kbd> to speed up).

4. The graphical images will start their corresponding desktop environment
and the graphical installer, which can take some time. The minimal images
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also the fact that some people use awfully bad USB pendrives during installation, and this can make the boot time takes a few minutes.

@thiagokokada
Copy link
Contributor

Why is this still not merged?

Everyone is complaining that the documentation is outdated or incomplete and this PR would improve that, but now it's stalled for about 2 weeks. I had plans for further improvements once this is merged, but this contribution experience is very discouraging.

Sorry, but commiters are also busy folks. And also we sometimes forget about PRs.

A reminder to review/merge a PR is always welcome, but please avoid an aggressive tone when doing so.

@fufexan
Copy link
Contributor

fufexan commented Jul 28, 2022

What's the status on this PR? It's been sitting for some time.

Copy link
Member

@bobvanderlinden bobvanderlinden left a comment

Choose a reason for hiding this comment

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

This does seem to be an improvement over what is currently in the manual. The instructions for Linux seem to be correct (which is the part I can currently verify). Seems fine to have this available in the manual already.

@davidak
Copy link
Member Author

davidak commented Oct 25, 2022

@bobvanderlinden thank you very much!

I rebased and built it again. Can we get another review and merge it? I don't want to miss another release.

@thiagokokada
Copy link
Contributor

@davidak Can you fix the two missing . that I found before? Otherwise LGTM.

@davidak
Copy link
Member Author

davidak commented Oct 26, 2022

@thiagokokada i don't think a list of bullet points need a period, but i can do it for consistency. some have them when there are multiple sentences

@thiagokokada
Copy link
Contributor

@thiagokokada i don't think a list of bullet points need a period, but i can do it for consistency. some have them when there are multiple sentences

Yeah, I was saying this mostly because only the first point on each was missing periods.

But I already fixed anyway, going to merge once I got back home.

- Update download URLs
- Replace "USB stick"/"USB Drive" with "USB flash drive" as that seem more correct

  https://en.wikipedia.org/wiki/USB_flash_drive
  https://elementary.io/docs/installation#choose-operating-system

- Don't mention CD as easiest option anymore,
  as all modern systems should be able to boot from USB,
  but many don't have a CD drive. Burning CDs is also usually wasteful as you
  can't burn them again.
- Remove link to NixOS Wiki (Making_the_installation_media) as it is not needed
- Add Etcher and USBImager as graphical tools to create install drive
- Make dd command consistent and use block size of 4 MB for faster flashing
- More consistent text
- Add instructions for "Booting from the install medium"

  Inspired by https://github.com/elementary/website/blob/9a91b0f4956b059db02122fbc738c4c067e033a7/docs/installation.md#booting-from-the-install-drive-booting-from-the-installation-medium-clear-float-2

- Add instructions for "Graphical Installation"
- Restructure headings and anchors for "Manual Installation"
- Adding legacy anchors for "Manual Installation" to not break links

Co-authored-by: j-k <dev@j-k.io>
Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Co-authored-by: Robert Schütz <github@dotlambda.de>
Co-authored-by: Jörg Thalheim <Mic92@users.noreply.github.com>
Co-authored-by: Thiago Kenji Okada <thiagokokada@gmail.com>
@davidak
Copy link
Member Author

davidak commented Oct 26, 2022

@thiagokokada sorry i did not get your point why that is useful.

I squashed your commit and added you to co-authors. I like that this is such a collaborative effort with 6 authors!

I also built it again, successfully.

@thiagokokada
Copy link
Contributor

@thiagokokada sorry i did not get your point why that is useful.

I squashed your commit and added you to co-authors. I like that this is such a collaborative effort with 6 authors!

I also built it again, successfully.

I meant, the problem for me was the inconsistency, not correctness. It was only the first point on two different sections that were missing periods, while the other points in the same section included them.

You could either remove all of them or add the periods in the two places that I indicated, that was it.

@thiagokokada
Copy link
Contributor

Let's merge this before 22.11 release cut, so folks have time to improve on it until the release is finished.

@thiagokokada thiagokokada merged commit 73ba4de into NixOS:master Oct 26, 2022
@thiagokokada
Copy link
Contributor

Thanks @davidak and everyone else for the improvements.

@davidak davidak deleted the doc-graphical-installer branch October 26, 2022 12:44
@github-actions
Copy link
Contributor

Backport failed for release-22.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-22.05
git worktree add -d .worktree/backport-175128-to-release-22.05 origin/release-22.05
cd .worktree/backport-175128-to-release-22.05
git checkout -b backport-175128-to-release-22.05
ancref=$(git merge-base b3c0c4979e3405f3b267e9fd1c81c4f82d8ba44d f701bd59860f4ea75ab0c20095fc8739b487875c)
git cherry-pick -x $ancref..f701bd59860f4ea75ab0c20095fc8739b487875c

@davidak davidak added the 8.has: port to stable A PR already has a backport to the stable release. label Nov 4, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2022

Backport failed for release-22.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-22.05
git worktree add -d .worktree/backport-175128-to-release-22.05 origin/release-22.05
cd .worktree/backport-175128-to-release-22.05
git checkout -b backport-175128-to-release-22.05
ancref=$(git merge-base b3c0c4979e3405f3b267e9fd1c81c4f82d8ba44d f701bd59860f4ea75ab0c20095fc8739b487875c)
git cherry-pick -x $ancref..f701bd59860f4ea75ab0c20095fc8739b487875c

gador pushed a commit to gador/nixpkgs that referenced this pull request Feb 13, 2023
since support for kbd elements was added with explicit intent in NixOS#175128
it seems like a good idea to support this in nixos-render-docs instead
of just dropping it in favor of `*F12*` etc. since it's a very rare
thing in the manual and purely presentational it makes sense to use
bracketed spans instead of a new myst role.

the html-elements.lua plugin is now somewhat misnamed, but it'll go away
very soon so we don't want to bother renaming it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation 8.has: port to stable A PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.