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

Seperated and Added to the Linux and Andriod Install Pages #38942

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alanakihn
Copy link

Resolves #38572

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

I separated the Linux and Android install pages as to provide more clarity for developers targeting these operating systems. I also added some additional information for more Linux distributions and a short guide on setting up the Android emulator. This is addressing the open issue: #38572.

Currently I have added the two files in the home directory as markdown files, as I don't have permission to edit the Wiki directly.

@alanakihn alanakihn requested a review from a team as a code owner June 11, 2024 03:19
@mihaiplesa mihaiplesa requested a review from wknapik June 12, 2024 10:12
@wknapik
Copy link
Contributor

wknapik commented Jun 13, 2024

Splitting the instructions for Android and Linux is reasonable, but so far instructions for all platforms are on the wiki, not in the repo. I'd rather keep it consistent.

The advantage of the wiki is that changes are quick and easy compared to opening a brave-core PR. But if we go for repo instead, why just two platforms?

If we decide to switch to the repo, we should replace the content of the relevant wiki pages with links to the markdown files in the repo.

If we decide to go with the wiki, I'm not sure if there's a way to have external contributions there.

@mihaiplesa @bsclifton - wdyt?

Other than the above:

  • I see no changes to the README.md file, which should point to the newly split instructions for Linux and Android
  • The two new files are placed at the top of the repo, with generic names - that should probably be adjusted to reflect what those files actually document
  • The Android file doesn't appear to contain information that was present (and relevant) on the existing combined wiki - system requirements, dependency installation, troubleshooting, known issues

Given that these are instructions for developers, I think this should be reviewed by someone on the dev team.

Copy link

@Chanchaichoochuay007 Chanchaichoochuay007 left a comment

Choose a reason for hiding this comment

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

Android.md

@@ -0,0 +1,57 @@

Choose a reason for hiding this comment

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

.


Alternatively, you can use Yarn. Follow the [Yarn install docs](https://yarnpkg.com/getting-started/install) to install Yarn and a compatible version of Node.js. After installing Yarn, run `yarn import` to create a `yarn.lock` file from `package-lock.json`.

## Additonal installs for different Linux distributions:

Choose a reason for hiding this comment

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

Suggested change
## Additonal installs for different Linux distributions:
## Additional installs for different Linux distributions:

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.

Improve Clarity of Linux/Android Section in README
5 participants