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

Fix deps in //brave/browser/importer #6664

Merged
merged 1 commit into from
Sep 26, 2020

Conversation

Gyuyoung
Copy link
Contributor

Below files have a strong dependency with chrome/browser.
So, this PR adds sources.gni to //brave/browser/impoprter,
then make it build in //chrome/browser.

  • brave_external_process_importer_client.cc
  • brave_external_process_importer_client.h
  • brave_external_process_importer_host.cc
  • brave_external_process_importer_host.h
  • brave_in_process_importer_bridge.cc
  • brave_in_process_importer_bridge.h

Resolves brave/brave-browser#11752.

Resolves

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

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.

@Gyuyoung Gyuyoung requested a review from bridiver as a code owner September 17, 2020 06:27
@Gyuyoung Gyuyoung force-pushed the gyuyoung-brave-browser-importer branch from ae04f15 to c938d11 Compare September 18, 2020 07:07
"task_manager/sampling/shared_sampler_posix.cc",
]
}
+ sources += brave_chrome_browser_importer_sources
Copy link
Collaborator

@bridiver bridiver Sep 21, 2020

Choose a reason for hiding this comment

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

since this is all part of //chrome/browser we don't want individual patches here. Please add //brave/browser/sources.gni (which will include //brave/browser/importer/sources.gni and do

brave_chrome_browser_sources = []
brave_chrome_browser_sources += brave_chrome_browser_importer_sources

and then change this patch to sources += brave_chrome_browser_sources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, fixed like that.

@Gyuyoung Gyuyoung force-pushed the gyuyoung-brave-browser-importer branch 4 times, most recently from e5e125e to 551c09c Compare September 23, 2020 05:56
"task_manager/sampling/shared_sampler_posix.cc",
]
}
+ sources += brave_chrome_browser_sources
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about the deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we talked on slack, I add the deps as well.

@Gyuyoung Gyuyoung force-pushed the gyuyoung-brave-browser-importer branch 2 times, most recently from 04f12d3 to f31664b Compare September 25, 2020 00:53
@Gyuyoung Gyuyoung added this to the 1.16.x - Nightly milestone Sep 25, 2020
@Gyuyoung Gyuyoung force-pushed the gyuyoung-brave-browser-importer branch 2 times, most recently from d9678cf to 7421f9f Compare September 25, 2020 03:40
Below files have a strong dependency with chrome/browser.
So, this PR adds sources.gni to //brave/browser/importer
and //brave/browser/sources.gni, then make
//brave/browser/sources.gni include //brave/browser/importer/sources.gni,
finally build //brave/browser/sources.gni in //chrome/browser.

  - brave_external_process_importer_client.cc
  - brave_external_process_importer_client.h
  - brave_external_process_importer_host.cc
  - brave_external_process_importer_host.h
  - brave_in_process_importer_bridge.cc
  - brave_in_process_importer_bridge.h

Resolves brave/brave-browser#11752.
@Gyuyoung Gyuyoung force-pushed the gyuyoung-brave-browser-importer branch from 7421f9f to 3f71361 Compare September 25, 2020 07:12
@Gyuyoung Gyuyoung merged commit da80b81 into master Sep 26, 2020
@Gyuyoung Gyuyoung deleted the gyuyoung-brave-browser-importer branch September 26, 2020 01:48
@Gyuyoung Gyuyoung mentioned this pull request Oct 20, 2020
33 tasks
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.

[Desktop] Fix deps in //brave/browser/importer/*
2 participants