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

Command to reset non-patched chromium files #5758

Open
wants to merge 1 commit into
base: master
from

Conversation

@petemill
Copy link
Member

petemill commented Aug 22, 2019

image

Shows a diff of each modified chromium file and asks if the user wants to reset that file. This makes sure only files that are modified are reset, if desired, and also gives an opportunity for the developer to not lose work unintentionally.

The main scenario this fixes and that I have used this command for:

  • Developer wants to reset all chromium files to the current brave-core patched state, but doesn't want to end up touching every patched file by performing a hard reset, which would result in a lengthy build. Right now, they could run git reset --hard in src directory (or run npm run init which will run that same git reset command), but that will result in a very long subsequent build time. Now, they can run both npm run sync and this new command, npm run reset_non_patched, will make sure only the few user-local-modified files (either that are patched by brave-core or others) are reset, and not the entire chromium repository.

Another scenario this fixes, though not perfectly:

  • A scenario whereby a developer can be editing chromium files in an attempt to debug an issue or create a feature, but doesn't know exactly which changes they wish to keep. Instead of running npm run update_patches and having to decide which patches to keep and dismiss and then resetting the other changes manually, this command will allow them to make the decision about which changes to keep or discard first, before committing to create patches.

Fix #5757

Test Plan:

This adds a new command and does not affect any feature code. Build and sync should still be tested since it edits libs used by those scripts.

If you would like to use the new command:

  1. Modify a chromium file (that is not already being patched by brave-core
  2. Run npm run reset_non_patched
  3. Observe that the script outputs a question asking if you want to reset each modified file. Entering Y should reset that file at the end, and N should not.

Submitter Checklist:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions.

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.
@petemill petemill requested review from bridiver and bsclifton Aug 22, 2019
@petemill petemill self-assigned this Aug 22, 2019
Shows a diff of each modified chromium file and asks if the user wants to reset that file. This makes sure only files that are modified are reset, if desired, and also gives an opportunity for the developer to not lose work unintentionally.

This fixes a couple of different scenarios:
1. Developer wants to reset all chromium files to the current brave-core patched state. Right now, they could run `git reset --hard` in `src` directory (or run `npm run init` which will run that same git reset command), but that will result in a very long subsequent build time. Running both `npm run sync` and this new command, `npm run reset_non_patched`, will make sure only files that are patched by brave-core, and files that were otherwise modified are reset, but not the entire chromium repository.
2. A scenario whereby a developer can be editing chromium files in an attempt to debug an issue or create a feature, but doesn't know exactly which changes they wish to keep. Instead of running `npm run update_patches` and having to decide which patches to keep and dismiss and then resetting the other changes manually, this command will allow them to make the decision about which changes to keep or discard first, before committing to create patches.
@petemill petemill force-pushed the reset-non-patched-chromium-files branch from 80f29ff to 162d823 Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

1 participant
You can’t perform that action at this time.