Updated README.md so to guide people batter about tags and branch checkout, I had to sacrifice 2 days work to figure this out

[NaaeemMalik]

Resolves

Submitter Checklist:

• [x] 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
• [x] 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)

Reviewer Checklist:

• [ ] A security review is not needed, or a link to one is included in the PR description
• [ ] New files have MPL-2.0 license header
• [ ] Adequate test coverage exists to prevent regressions
• [ ] Major classes, functions and non-trivial code blocks are well-commented
• [ ] Changes in component dependencies are properly reflected in gn
• [x] Code follows the style guide
• [ ] Test plan is specified in PR before merging

After-merge Checklist:

• [ ] The associated issue milestone is set to the smallest version that the changes has landed on
• [ ] All relevant documentation has been updated, for instance:
  • [ ] https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • [ ] https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • [ ] https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
  • [ ] https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • [ ] https://github.com/brave/brave-browser/wiki/Custom-Headers
  • [ ] https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
  • [ ] https://github.com/brave/brave-browser/wiki/QA-Guide
  • [ ] https://github.com/brave/brave-browser/wiki/P3A
  • [ ] https://github.com/brave/brave-browser/wiki/Brave-Wallet

Test Plan:

[NaaeemMalik]

Updated README.md so to guide people batter about tags and branch checkout, I had to sacrifice 2 days work to figure this out. without this documentation change other people will also find it very hard to contribute. I confirm nothing other then README.md and author name in package.json is changed

[NaaeemMalik]

can I know when will it get approved, its my first open source contribution Sir

[bsclifton]

Thanks for taking the time to submit this

We already have the information about installing dependencies here: https://github.com/brave/brave-browser/wiki/Linux-Development-Environment

For the checkout, I think what we have is OK. I'm not sure why you had to run this twice. We've basically removed all the important stuff from brave-browser and you should only ever need to get the tags from brave-core. You can run npm run sync from inside src/brave instead of being at the brave-browser root

[NaaeemMalik]

yeah I think tags is my bad then, I was on root and running sync throw that error, I didnt expect tags on this repo releases are needed to be checked-out in other repo instead. so maybe we should instead write

brave-browser/src/brave> git fetch origin
brave-browser/src/brave> git checkout [-b] branch_name [tags/tag_name]
brave-browser/src/brave> npm run sync
...Updating 2 patches...
...Updating child dependencies...
...Running hooks...

so its descriptive enough. before > we should write local path not repo path bcz > means its path on local machine and people will look for directory there.

and about Linux dependencies, its hard to remember that you have to install them too so main page should mention this step a bit since Linux page only contains this info and its a must do and all instructions about android already exist here except this one

[NaaeemMalik]

I also want to mention it needs 65GB internet data as a requirement to help alot of people who are on mobile data and dont know how big internet package they need in a new commit

[NaaeemMalik]

so should I do these commits?

[bsclifton]

@NaaeemMalik maybe we can update the section that looks like this:

brave-core> git checkout -b branch_name

to instead have the path- like you're saying

src/brave> git checkout -b branch_name

For calling out the size, we do have this note:

# the Chromium source is downloaded, which has a large history
# this might take really long to finish

We can edit this to give an idea of how large it's is in size - (gigabytes). It's possible to omit the git history when fetching (which cuts it down significantly), but that's not an option - we need the full history. Any browser using Chromium will have this same problem too, unfortunately (compiling it all can take hours too, depending on your machine).

It's worth noting the Install prerequisites pages link to the Chromium docs which have the system requirements:

• macOS (linked from https://github.com/brave/brave-browser/wiki/macOS-Development-Environment)
• Windows (linked from https://github.com/brave/brave-browser/wiki/Windows-Development-Environment)
• Linux/Android (linked from https://github.com/brave/brave-browser/wiki/Linux-Development-Environment)

Let's completely remove the Install Dependencies on Linux section from your PR, because we have the Install prerequisites section which covers macOS, Windows, and Android in addition to Linux.

Thanks for taking the time to look at this. We typically create issues before making changes here - that's a good way to capture what change you're trying to achieve before you start any work

[NaaeemMalik]

Sorry I was confused how to do since its not issue inside browser, just love Brave so much that I wanted to contribute at any cost

about prerequisites I get your point, but we both are from different background, you are from USA :) and I'm from Pakistan and you likely never had to face problems like I'm mentioning below, first freelance project I had to do on Brave was when I had 4 core 8GB memory laptop on 8mbps mobile data last year. prerequisites are something that are needed before doing something, but this is kind of in-the-middle requisite. it take npm run init step about 12 hours on my 16Mbps connection, if you are on slower connection it gets cancelled again and again and you have to delete everything and redo everything from clone step, sometimes it takes days/weeks to download chromium code, this step also needs huge memory so people cant keep 2 tabs open for so long, and after so much hours/days nobody remembers that there was a in-the-middle requisite on a page that is named as prerequisites and he obviously wont open that page so we shouldn't consider this step a prerequisite, so either we should name link text to pre & post requisites or just mention the step little bit again, and chromium has also created new Install additional build dependencies section instead of writing it inside prerequisites section just like me and people again cant keep all these links open that you have mentioned above after download has started. but I will remove it if you want.

also how about we also write

git clone [email protected]:brave/brave-core.git path-to-your-project-folder/src/brave

as

git clone https://github.com/brave/brave-core.git path-to-your-project-folder/src/brave

to help people quickly & easily set it up without first needing to setup GitHub with SSH? isn't HTTPS secure enough?

[NaaeemMalik]

and brave.com is blocked in Pakistan and might be in some other countries as well, but those people can open github.com only, and not even people on reddit know that it can be downloaded from https://github.com/brave/brave-browser/releases/ and they are suggesting third party websites which can have malware in them. so should I mention that link in downloads section? people here are also unable to install extensions in Brave since I think brave needs to make connection to brave.com to get proxy data but it cant receive that so it says extension failed to install but I will create issue for that and discuss there and send PR when I get time

[NaaeemMalik]

Sir this is how I think it should be, you are batter then me, you can probably just make it look like how you you want it to be and then approve

[bsclifton]

Thanks for the updates - and sorry it's taken a while to look at this ☹️ Will try to review soon

[NaaeemMalik]

Hi Can you just hit approve, it's been 3.5 weeks

[NaaeemMalik]

Anyone?

[bsclifton]

@NaaeemMalik if you read the contributing guidelines and docs, we share that the best practice is to create an issue describing what you'd like to change and then link to it.

I appreciate your enthusiasm, but we don't just hit merge on anything that comes because the person wants us to hit merge

I'll pull your branch right now and do some cleanup

[bsclifton]

Did some cleanup - squashed it all down to one commit and merged 👍

[NaaeemMalik]

Thankyou <3

[NaaeemMalik]

that makes you my first open source mentor 🥰

[NaaeemMalik]

I am sure these changes will help alot of people new to this project