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

Update installation section #207

Merged
merged 6 commits into from
May 29, 2024
Merged

Update installation section #207

merged 6 commits into from
May 29, 2024

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented May 29, 2024

preview

Description of proposed changes

See commit messages

Related issue(s)

070da40 and 29f44b9 were prompted by debugging in the lab. The rest is stuff that I thought of while re-reading the contents.

Checklist

  • Checks pass

@victorlin victorlin self-assigned this May 29, 2024
@victorlin victorlin changed the title Update Nextstrain CLI installation section Update installation section May 29, 2024
@victorlin victorlin marked this pull request as ready for review May 29, 2024 21:58
@victorlin victorlin requested a review from a team May 29, 2024 21:59
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

+1 for this polish overall.

I noted that the commits don't particularly say why the change is made. Like, I can hazard some guesses about why the start menu wording was changed (e.g. maybe there are other results for wsl that aren't the right one to pick?), but the reason is definitely not clear. It's really useful for the commit message to contain the why.

src/install.rst Outdated Show resolved Hide resolved
src/install.rst Outdated Show resolved Hide resolved
victorlin added 3 commits May 29, 2024 16:18
It's possible for Ubuntu to not be the default distro. In this case, the
error of `curl: not found` can be unhelpful to new users. Add a note
explaining how to fix it.
Use the guilabel directive and add names for Windows.
Windows 10/11 comes with a search bar/icon in the task bar that opens
the Start menu, so if the user doesn't know what "Start menu" is, the
verb "searching" should provide a visual hint.
@victorlin victorlin force-pushed the victorlin/update-install branch from 7f3f5a7 to cdc6374 Compare May 29, 2024 23:22
victorlin added 3 commits May 29, 2024 16:23
Some users may not have Launchpad in the Dock - it can be easily removed
by drag and drop. Spotlight in the menu bar is more universal¹, plus
users who know the hotkey (CMD+space) can use it if desired.

¹ it can still be removed with 3rd party apps
This should be a more intuitive flow for new users.
The previous recommendation was to expose the Windows home directory
under the WSL directory. I thought this would be a practical approach to
the difference in home directories, but I'm not a WSL user so I can't
verify that idea. Installing `wslu` is also a bit more complicated
nowadays.¹

Instead of a hard recommendation, simply describe the difference in home
directories and let the user decide what to do.

¹ <https://wslutiliti.es/wslu/install.html#ubuntu>
@victorlin victorlin force-pushed the victorlin/update-install branch from cdc6374 to 29f44b9 Compare May 29, 2024 23:24
@victorlin victorlin merged commit 226470c into master May 29, 2024
4 checks passed
@victorlin victorlin deleted the victorlin/update-install branch May 29, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants