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

Updated macOS instructions for 10.15 (Catalina) #632

Merged
merged 4 commits into from
Dec 19, 2019

Conversation

LHurst-UoB
Copy link
Contributor

Bash is not default any more, it's now zsh. Used Linux instructions as template for telling users how to run it.

Bash is not default any more, it's now zsh.  Used Linux instructions as template for telling users how to run it.
@fmichonneau
Copy link
Contributor

Could someone from @carpentries/lesson-infrastructure-committee with Catalina confirm that these instructions are accurate?

@mikerenfro
Copy link
Contributor

Could someone from @carpentries/lesson-infrastructure-committee with Catalina confirm that these instructions are accurate?

I'm not from the infrastructure committee, but the Mac instructions at http://swcarpentry.github.io/shell-novice/setup.html have had Catalina language in them for about a month. I don't see anything wrong with the instructions in this PR, but you could also borrow language from the setup page instead.

@LHurst-UoB
Copy link
Contributor Author

For what it's worth - if the language needs adjusting for this then the Linux instructions also need changing since I just copied and pasted (substituting Mac OS for Linux) from the instructions provided for Linux users without bash as their default shell.

@mikerenfro
Copy link
Contributor

For what it's worth - if the language needs adjusting for this then the Linux instructions also need changing since I just copied and pasted (substituting Mac OS for Linux) from the instructions provided for Linux users without bash as their default shell.

In the original PR comments, there was a rationale for leaving the Linux instructions alone, as all the distributions I checked used bash by default.

@maxim-belkin
Copy link
Contributor

Um... some Linux distros use Dash by default

@mikerenfro
Copy link
Contributor

For scripts using #!/bin/sh, yes. But that’s not a default interactive shell, and any scripts in these lessons are explicitly run with bash scriptname.

@maxim-belkin
Copy link
Contributor

But that’s not a default interactive shell

Oh, so Dash is used for non-interactive shells and Bash for interactive ones... good to know! Thank you!

@LHurst-UoB
Copy link
Contributor Author

LHurst-UoB commented Dec 19, 2019

In the original PR comments, there was a rationale for leaving the Linux instructions alone, as all the distributions I checked used bash by default.

I wasn't aware of that change - why are we duplicating instructions in separate repositories (which are now out of sync, it seems)?

Regarding "leaving the Linux instructions alone" - the Linux instructions in this repo already include exactly what the other pull request said not to include (these instructions for running bash if it is not the default). My point is that that this PR is an almost word-for-word copy of the existing instructions in the same file in this repo for Linux users so if this wording is wrong for MacOS it follows the existing wording I copied (for Linux users whose default shell is not bash, a few lines away in the same file) also need reviewing.

Added how to detect if bash is not the default shell and then run it.
@LHurst-UoB
Copy link
Contributor Author

LHurst-UoB commented Dec 19, 2019

just to note the checks are failing with /usr/local/bin/build: line 63: jekyll: command not found - I think that might be an infrastructure issue rather than my changes?

Merged interim changes from gh-pages to my fork has fixed it.

@LHurst-UoB
Copy link
Contributor Author

I'm not from the infrastructure committee, but the Mac instructions at http://swcarpentry.github.io/shell-novice/setup.html have had Catalina language in them for about a month. I don't see anything wrong with the instructions in this PR, but you could also borrow language from the setup page instead.

I've replaced both the macOS and Linux instructions here with similar language. I'm now concerned about the duplication of information as collectively we're failing to keep the setup instructions in both places in sync - I think I'll open an issue about that, so we can keep this PR focused on getting the macOS instructions up to date.

It's slightly embarrassing how long after Apple changed the default we are still giving attendees the wrong instructions on the front page of workshops.

@LHurst-UoB
Copy link
Contributor Author

For scripts using #!/bin/sh, yes.

#!/bin/sh is often not bash these days (there was a time many distros symlinked /bin/sh to /bin/bash), and if bash is invoked as 'sh' it has always entered "sh emulation mode" which disables many of it's advanced (and desirable) features. This relates to swcarpentry/shell-novice#927 which is a quagmire of an issue and, essentially, boils down to "is shell-novice a bash course or a sh (the POSIX shell) course?" - a question that a clear decision has not yet been reached.

@netlify
Copy link

netlify bot commented Dec 19, 2019

Deploy preview for zen-ride-08ffd1 ready!

Built with commit 0625f10

https://deploy-preview-632--zen-ride-08ffd1.netlify.com

Copy link
Contributor

@brownsarahm brownsarahm left a comment

Choose a reason for hiding this comment

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

I think this is a good change, but I don't have mac or anyone around me with Catalina to personally view it working. @mikerenfro's point that these instructions have been successful in a lesson's setup page is good enough for me

@mikerenfro
Copy link
Contributor

@mikerenfro's point that these instructions have been successful in a lesson's setup page is good enough for me

Not certain about successful, but at least painstakingly reviewed and approved over at the setup page. I won't get to run my first workshop until January.

@fmichonneau
Copy link
Contributor

Thank you all for these changes!
@LHurst-UoB note that we have carpentries/maintainer-RFCs#4 where we'd welcome your feedback on how best to deal with installation instructions for SWC (and LC) workshops.

@fmichonneau fmichonneau merged commit f671ff8 into carpentries:gh-pages Dec 19, 2019
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.

5 participants