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 setup.sh #290

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

justinberi
Copy link

./setup.sh fails if apt update has not been issued previously (ie from a fresh docker container).

@jcnorby jcnorby changed the base branch from main to devel June 9, 2022 13:43
@jcnorby jcnorby changed the base branch from devel to main June 9, 2022 13:44
@jcnorby
Copy link
Contributor

jcnorby commented Jun 9, 2022

This is a good update, and certainly something that needs to be done prior to installing the repo. We will likely add it to the installation instructions since it is important that the update succeeds without failing, and simply adding it to this script may result in new users not catching that something failed.

@jcnorby jcnorby added the bug Something isn't working label Jun 9, 2022
@jcnorby
Copy link
Contributor

jcnorby commented Jun 9, 2022

Oops, looks like our CI setup doesn't like PRs opened by external users - we'll look into that.

@justinberi
Copy link
Author

No worries would you like me to close the PR and open two issues for now?

  • Documentation update
  • External PRs cause CI to fail

@jcnorby
Copy link
Contributor

jcnorby commented Jun 10, 2022

I already opened an issue for those changes - let's leave this PR open for now since ideally making those changes would enable this to go through. If we change the workflow we can close it later. Thanks!

@justinberi
Copy link
Author

Hey, I added some other minor refactors to the PR. Happy to revert if you wish to have only one commit in here.
Refactors include:

  • speed improvement
  • setup.sh exits on error (it was not doing this previously which caught me out)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants