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

Improve onboarding #1276

Merged
merged 9 commits into from
Jan 16, 2020
Merged

Improve onboarding #1276

merged 9 commits into from
Jan 16, 2020

Conversation

rickycodes
Copy link
Contributor

@rickycodes rickycodes commented Jan 15, 2020

Description

Changes to improve on-boarding for the next human that comes along

List of items to address:

  • env files
    • add missing example files (I think android?)
    • add check for env files in build.sh
  • Sdk requirements for Android
    • sdk.dir needs to be defined in local.properties and/or $ANDROID_HOME needs to be defined in $PATH (~/.profile, or ~/.bash_profile etc.)
  • secret-tool is a requirement
    • [ ] test for this eg: which secret-tool in the build.sh
  • licence needs to be accepted, this is easiest to accomplish with android studio
  • document all of the above in the README.md
  • [ ] step through changes in a VM to verify

@rickycodes rickycodes marked this pull request as ready for review January 15, 2020 19:33
@rickycodes rickycodes requested a review from brunobar79 January 15, 2020 19:33
Copy link
Contributor

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for doing this Ricky!

Can you add one more note in the read me mentioning that you need to rename the .env.example files first?

After that I'm okay with merging it and see if the CI build passes when merged into develop.

The reason why it might not pass is because the CI machine is macOS based and I've problems with certain shell syntax before where some of it works in linux only or macOS only.

if it doesn't we can revert and fix.

@rickycodes
Copy link
Contributor Author

Can you add one more note in the read me mentioning that you need to rename the .env.example files first?

done!

The reason why it might not pass is because the CI machine is macOS based and I've problems with certain shell syntax before where some of it works in linux only or macOS only.

Usually I try to use POSIX sh syntax which can be a little harder to work with, but in some linux there is no bourne again shell (or the version is older than what ships with OSX). For example I know there is a mismatch between OSX and Alpine linux (which a lot of build systems use because it's lightweight). However I think my changes should be okay in this case.

@brunobar79 brunobar79 merged commit c6cc8e6 into MetaMask:develop Jan 16, 2020
brunobar79 pushed a commit that referenced this pull request Jan 16, 2020
rickycodes added a commit that referenced this pull request Jan 31, 2022
* Create variables for env files

* Check for env file before building

* Add android env example

* Be a bit more specific as to where to add the env files

* Add envFileMissing func

* Double quote to prevent globbing and word splitting: https://github.com/koalaman/shellcheck/wiki/SC2086

* Update README with some additional details for running the app on android

* Do not commit fabric.properties

* Leave this alone
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.

2 participants