Allow creating PullRequest on a different repo than pushing #120

Yannic · 2020-02-29T22:13:50Z

No description provided.

mikelalcon · 2020-03-02T12:36:07Z

java/com/google/copybara/git/GitModule.java

@@ -1199,6 +1200,18 @@ public GitDestination gitHubDestination(
            named = true,
            doc = "Destination reference for the change. By default 'master'",
            defaultValue = "\"master\""),
+        @Param(
+            name = "pr_destination_url",


Really nice change! I've been wanting to do this for ages :) . Could we use the current user fork by default? In that way the config is stable and doesn't change between who is running it. Maybe something like: pr_to_fork = True. If there is still the need for doing it for arbitrary repo, we can also have the param you have, and decide based on what fields are set.

I could imaging having push_to_fork = True that changes the "owner" part of the url to the current user (I think the username is already available, right?). Setting fork_url manually is then only required if the fork has a different name, or when pushing on behalf of another user or to an organization.

destination = git.github_pr_destination( url = "https://github.com/google/copybara.git", push_to_fork = True, # Defaults to False. fork_url = "https://github.com/Yannic/copybara.git", # Defaults to "https://github.com/<current_user>/copybara.git". )

Pointing url to the fork and detect the upstream might also work, but, AFAICT, copybara doesn't know the upstream url yet, and the config also would have to change based on who is running it.

hsudhof · 2020-03-10T21:43:50Z

docs/reference.md

@@ -2140,6 +2140,7 @@ Parameter | Description
 --------- | -----------
 url | `string`<br><p>Url of the GitHub project. For example "https://github.com/google/copybara'"</p>
 destination_ref | `string`<br><p>Destination reference for the change. By default 'master'</p>
+pr_destination_url | `string`<br><p>Url of the GitHub project to create the PullRequest on. Set this if you want to push to a personal fork and create the PullRequest on the upstream project (e.g. because you don't have write access to the upstream repo).By default, `pr_destination_url` is the same as `url`.</p>


Suggested change

pr_destination_url | `string` Url of the GitHub project to create the PullRequest on. Set this if you want to push to a personal fork and create the PullRequest on the upstream project (e.g. because you don't have write access to the upstream repo).By default, `pr_destination_url` is the same as `url`.

pr_destination_url | `string` Url of the GitHub project to create the PullRequest on. Set this if you want to push to a personal fork and create the PullRequest on the upstream project (e.g. because you don't have write access to the upstream repo). By default, `pr_destination_url` is the same as `url`.

Yannic · 2020-03-20T18:08:38Z

I update the PR to use @mikelalcon's design. PTAL, thanks!

Included changes: - dc080e95161964a1ff841bfd0b871a1123c027a8 Prettify failure message of asserts.set_equals. (#263) - 16de038c484145363340eeaf0e97a0c9889a931b Add missing bzl_library for analysis_test.bzl (#262) - d62d6f52d5a3937b423d9aa755c8f60ea5e04dd5 rm last usage of --experimental_build_setting_api (#260) - 07922b040c3d8b8b0317e3dc687db625dd3e3cd4 Stop depending on rules_pkg through the federation. (#259) - 2a44ef8ec8c419abd49852b873209de3ab742af7 Add `new_sets` as a dep of `sets` bzl_library (#253) - 8f3151fb4a91d5f2ae4cad5901ea72fe30a2aba0 copy_file: Add parameter to allow symlinks (#252) - d35e8d7bc6ad7a3a53e9a1d2ec8d3a904cc54ff7 Create Gazelle language for Starlark (#251) - b10f2cb0fc013218f727de8c631d6dd1cfc078ea Remove --experimental_build_setting_api usage (#249) - ec2170289f3845a40945e9fbdfed03bddad13a90 Add myself to codeowners for rules (#256) - 3b666f525dfc645eea2bc4832796f8d6a07a997a Address lint errors (#254) - 560d7b2359aecb066d81041cb532b82d7354561b Add license and copyright notice (#245) - feb52960ebd8797421b599194ad6ac7da3fc7600 Fix diff_test when filepath includes external (#241) - 9935e0f820692f5f38e3b00c64ccbbff30cebe11 Depend on bzl_library, not on individual bzl files (#244) - 2d0c6512910452f2c8660466ed0dcefb642adea3 Update visibility of files (#243) - dfcfe825005ffba4a7c15cd9ddac737d040d2506 Fix type parsing errors on "always true" conditions. (#239) - 2d620ba1f8284695181fa092ec4064724ea99a9a Fix the comment to match the code. (#238) - 6970e21d290ceaa36502d0c94533b26e5ec18c0b Create a helper rule for selecting a file from outputs of... - add8e42934d1bf63ecf27f5439574383e74ef8fc Run buildifier over the directory. (#235) - 0a934a997d141ced0d8781f988da07ebe2101df0 Fix buildifier issue failing on CI. (#234) - 2d4c9528e0f453b5950eeaeac11d8d09f5a504d4 Add absolute path tests for Windows (#230) - 1c5ed0706a8e2f73116e128184be64a99e66e83d Migrate code for the flag --incompatible_disable_depset_i... - e583e822a0bb99e04cd4a2b9a3baf5e1088a214d Remove old_sets.bzl (#231) - 4b25373d12887f5add565197c4a163e9f1d9b716 Remove flag --incompatible_remap_main_repo (#227) - 327d61b5eaa15c11a868a1f7f3f97cdf07d31c58 Migrate for --incompatible_use_platforms_repo_for_constra... - 9f63e1a8d46d49ec44c007a2d43ba2f3bec84f7a make select_tests resilient to default configuration diff... - 930b3527b91213940f57289bb581a8b64d3c31ee Remove links to maprules (#213) - 734652aa5e1585646e90dfa19add0d57a954ce56 Remove unnecesssary license BUILD comment (#209) - e59b620b392a8ebbcf25879fc3fde52b4dc77535 Fix execute bit on empty_test.sh (#206) - cff8af42e9dd84bb7e4a9a89732c7c34fde7a75d Remove genfiles_dir retrieval method (#203) - 47c6eb15c6b16421b4a16be29bc1e42b80dcd559 Fix to 1.0.1 - add missing .bzl files (#201) - 376680d27667c09f4ff1021fa3fc442e02819031 Expose target_under_test's bin and genfiles path (#202) - 0e9da0dd08b763a5e3eece10a5abba5df6b8e46f fix distribution so that we get the right dependencies lo... - e5cf5398cd3e01d5552ed1056aafc06f92adb374 Update version and changelog in prep for 1.0.0 release (#... - f1475299afd41815c78c3768253468ea622669f3 Avoid some repetition in _make_analysis_test (#197) - 720f59405d95c5052a8e16762a8fd2a4d50baded Pass `fragments` through to analysis tests rule construct... - d8387f7bd0ef68f2939bf8898c73aa33d11af1a1 load rules thorugh maybe so we can simultaneous updates (... - 3154dbbc41da755a345afd8545e3152726e17c16 Add types.is_set() to test whether an arbitrary object is... - 58068fe0cc28d8d5e0115bf0e7d80189628f35db Delete maprule. Fix Buildifier lint errors. (#192) - 90ea6feaf33e8ef12fdac9981b2efccc6a25c727 Update "Getting Started" instructions to 0.9.0 release. (... - 3f7b196d2a57194ef025e32509e5966d73c5ea15 Update selects documentation. (#170) - ab87410a8bbfbe7306e21365d56461a20f5f4154 Use rules_pkg to make the skylib tarball for a release. (... - c5bd012f3119a4da13d523d47c36719ec288a86f add aiuto as code owner. direct changes to distribution/ ... - 6214b316195d2e37414814a2a84bb5e12e7362a7 Scope buildkite badge to the master branch (#186) - 143c602a1356512436e4830865f18be452119a99 Add jin to codeowners (#175) - f130d7c388e6beeb77309ba4e421c8f783b91739 Comply with the standards of the Bazel federation (#182) - d2cf1cc2bcd1e879743faf5216c4887b994705af `print`->`fail` and suppress the warning in another case.... - a6c72f2b390d4c574fbf75c2c325d926c4c48cd6 Make config_setting_group visibility aware (#168) - 2b38b2f8bd4b8603d610cfc651fcbb299498147f fix formatting problem (#169) - 10851c2c5bd528a518af23b2a23f2efe4427f139 Give BuildSettingInfo's value field a description (#167) - 21ee269a55222df15f49f0cec3bca997de292a66 Create new stardoc target for common_settings.bzl (#166) - c102f5d414bd2e019e2ac6104abd0ca1e244cd04 Get docs working again. (#165) - b113ed5d05ccddee3093bb157b9b02ab963c1c32 Create common build settings (#154) - 6a6a509f367ca0446e75c6f881719f5da2a55a26 selects.bzl: Add config_setting_group for config_setting ... - 446fd595bf1bb85768d885ab3a92dc8f80b29475 README.md: Fix typo in project name (#161) - efd3bfd0f9dea52ecf765f97a4708a89483943f7 Add some comments to unittest_test.sh (#159) - 202db59eccd13c5c9a98b4c905ed300eb2785e10 Make sets.bzl point to new_sets.bzl instead of old_sets.b... - 5c80706f70186507a262ef756fff80b2fde743e9 Fix for --incompatible_no_support_tools_in_action_inputs ... - 9aa308e1ef0f0f0543e3d58f3b3c5488c087c8c1 Fix repository for compatibility with --incompatible_no_s... - bf8a55b6687fc93c003d07505d886bbd1a7ff39a run_binary: runs an executable as an action (#153) - c5852220719bc29fdb256612adc4adde26b30b6c New rules: native_binary and native_test (#152) - 6126842e3db2ec4986752f6dfc0860ca922997f1 copy_file: expose the copying logic (#151) - 6bf64439752b2a98418a731ff21eb9d58ed5631c write_file: support different line endings (#150) - 67ecd632735a45267da051003d7eaf8d8ca1b998 Minor formatting changes plus doc updates (#147) - 84a12d1084bd156b4c0ed3f313299d2afdb5fe43 Fix a number of misc issues to allow google usage of baze... - 67215655bf6ce349b2b88ae4f5945a706f8ce959 Use TEST_SRCDIR for shell tests (#145) - ffad33e9bfc60bdfa98292ca655a4e7035792046 Pass through an attribute arguments to `analysistest.make... - f80abf6578612d4b5dda3dd50337d22c8ac7b37d Fix typo (#142) - 31b8ea5ea16f7bf9e81eb18a9f4cbd9b0a70b4d7 Add licenses() to all BUILD files. (#141) - be3b1fc838386bdbea39d9750ea4411294870575 diff_test: add rule and tests (#136) - 184c66e563babf7f6396c0d6236e75ed3a7bfcc1 Fix installation instructions (#135) - cc6a6c3ed4298d2a9fd5791b8d3546dee4f86d3f Fix broken link to new_sets doc page (#120) - 2b1c4610c3d26f74c3ca4a2074cb1115535dcaf0 Reformat with buildifier --warnings=all (#138) - 084758ff75463750f6c2476348ed9442e7860275 Regenerate docs + remove maprule.md (#137) - 4c26bf475c5cacade5367c92ef2dab5abe912112 Windows: fix tests for native test wrapper (#129) - 98ef48ebb2248e24184712ba3189de241707cc8a maprule: move functionality to maprule_util.bzl (#132) - 3721d32c14d3639ff94320c780a60a6e658fb033 maprule: hide it, not ready for public use. (#133) - aeefb6531aa30f678f0d8d40c6b5bf095408cdb5 Accept kwargs in dicts.add() (#130) - bdbedc18326673cca7b0ceab1b88719a35be3030 maprule: add basic integration test (#131) - b2dc5c0e63efdcfaddc0b4a4ecd8b0f8bea07978 e2e tests: make them run on Windows (#121) - 2d1669ed882652e5fc695bb99c77442a9273ff56 write_file: add rule and tests (#122) - db27394846f9beda839c10fa1e37a00a2a193235 copy_file: add rule and tests (#123) - f26e8ac863e5fc764189853723cd3e707fc039a9 Use //... to mean "all packages" (#128) - b2b4471332abdbf8979c89ca5e2cbb4f3a2bdc0f run buildifier 0.22.0 (#125) - bb56986cd4b446f6ec26fdb259c320ca95677303 Fix Buildifier version (#124) - 1b28145983091cbe8639d2816fb03ccb3950bbc9 maprule: use ctx.resolve_tools (#117) - 9630853eebaee22d8799f97ab58f648a1b82d02d add documentation pages for rules/ and lib/ (#119) - e171ec16d89afe28d09d29837ce523aa10aebdc6 CI updates. (#118) - a2ec47917b52199de11b737e4197e9fd040b4bfd fix a number of warnings found by Starlark analyzer (#114) - a35b13a1c9a3d43e81fcfaca42155c53d4b31cb7 analysistest API for retrieval of actions registered by t... - a5655124382ccbf7214456468cec6b7ec2e39c41 CI simplification/improvements for buildifier (#113) - baaef76aaa16bd1230de949d620e6a40fe26cba6 Add analysis_test rule - 6bb8994a03e9e47b9394888d82444427c5f6149e Add analysis-test test framework (#110) - aa3147f0de51c645aefcf82e523a4ccd0f85cb3c Create CODEOWNERS (#103) - 7a536d396bb1995d6eec660a675f45cef89b0f12 Add basic shell testing for unittest.bzl (#108) - 6741f733227dc68137512161a5ce6fcf283e3f58 Add `types.is_depset`. (#105) - 4b67f5ff384d1d2b0925299e0c1919d36b4ecaab Add rules to the test_deps target. (#102) - 97dc99e8a6f3cf216e8b4eb95b830ef43118cabb Fix example load directive. (#101) - d3d5ba7d05c60ecf91ecd8c9db30d4c7e2381947 Some doc fixes (#100) - f5e50bc53c52708abd29919a66bac307c6dc2e07 Add a build_test rule. (#97) - 9b85311ab47548ec051171213a1b3b4b3b3c9dc8 Enable tests on Windows (#99) (And 71 more changes) BAZEL_SKYLIB_VERSION_REV_ID: dc080e95161964a1ff841bfd0b871a1123c027a8 Change-Id: I1814ae3eaf4056f9f71be03247731558244304e4

SaptarshiSarkar12

@Yannic please resolve the branch conflicts. What is the condition of this PR? Is it ready to be merged?

googlebot added the cla: yes label Feb 29, 2020

autoyannic force-pushed the B625ED9427F37E62A3F643AC50AE48C3 branch from fe276db to 7a51f8b Compare February 29, 2020 22:19

mikelalcon reviewed Mar 2, 2020

View reviewed changes

hsudhof reviewed Mar 10, 2020

View reviewed changes

autoyannic force-pushed the B625ED9427F37E62A3F643AC50AE48C3 branch 11 times, most recently from 1a94a62 to 8c3c62d Compare March 12, 2020 14:33

Export of internal change

ae4c487

autoyannic force-pushed the B625ED9427F37E62A3F643AC50AE48C3 branch from 8c3c62d to ae4c487 Compare March 20, 2020 17:01

SaptarshiSarkar12 reviewed Sep 16, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow creating PullRequest on a different repo than pushing #120

Allow creating PullRequest on a different repo than pushing #120

Yannic commented Feb 29, 2020

mikelalcon Mar 2, 2020

Yannic Mar 2, 2020

hsudhof Mar 10, 2020

Yannic commented Mar 20, 2020

SaptarshiSarkar12 left a comment

	pr_destination_url \| `string`<br><p>Url of the GitHub project to create the PullRequest on. Set this if you want to push to a personal fork and create the PullRequest on the upstream project (e.g. because you don't have write access to the upstream repo).By default, `pr_destination_url` is the same as `url`.</p>
	pr_destination_url \| `string`<br><p>Url of the GitHub project to create the PullRequest on. Set this if you want to push to a personal fork and create the PullRequest on the upstream project (e.g. because you don't have write access to the upstream repo). By default, `pr_destination_url` is the same as `url`.</p>

Allow creating PullRequest on a different repo than pushing #120

Are you sure you want to change the base?

Allow creating PullRequest on a different repo than pushing #120

Conversation

Yannic commented Feb 29, 2020

mikelalcon Mar 2, 2020

Choose a reason for hiding this comment

Yannic Mar 2, 2020

Choose a reason for hiding this comment

hsudhof Mar 10, 2020

Choose a reason for hiding this comment

Yannic commented Mar 20, 2020

SaptarshiSarkar12 left a comment

Choose a reason for hiding this comment