Many WATCHLIST and codereview.settings files contain
large public email lists, which defeats the purpose
of private reviews.
R=tandrii@chromium.org
Bug: 721880
Change-Id: Ibb3f314f735b783e628f8cc1fc22c74e0df59f1c
Reviewed-on: https://chromium-review.googlesource.com/506489
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Before, git cl creds-check expected `http.cookiefile` of git config
to have path without '~' which git understands on Linux/Mac.
R=machenbach@chromium.org
Bug: 719728
Change-Id: Ica1308b776b62bda7a8b85113c0c0df837c7d933
Reviewed-on: https://chromium-review.googlesource.com/501828
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
The old system had two faults:
* It set reviewers and ccs via different mechanisms, which is confusing
* It set CCs with a single call for each, resulting in N separate emails,
with each email going to the uploader and all reviewers and all previous
CCs.
This new system just collects all reviewers and CCs, and sets them
in a single call. That call will fail if *any* of the individual
reviewers or ccs fail, so it also parses the response and retries with
only the ones which would have succeeded on their own. If that second
call fails, or the first fails in an unexpected way, it raises an
exception like normal
Bug: 710028
Change-Id: I1be508487a41f0b68f9c41908229b8f5342830a3
Reviewed-on: https://chromium-review.googlesource.com/479712
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Previously, if the branch structure was
origin/master
master [ 3 commits behind origin/master ]
my-branch [ 1 commit ahead of master ]
then git-cl would see that the upstream branch is master,
and decide to diff against origin/master instead. But that
diff would contain the contents of the three commits that
the local master branch hasn't caught up with yet. That's
not what we want.
This CL makes it so that basing on top of local master is
the same as basing on top of remote branches. Just like
when based on top of origin/master, it simply calculates
the merge-base with the parent commit (which is assumed
to be present in the remote), and uploads that diff.
Change-Id: I91caa30f6256a6e019e613dc5ff624734bca027e
Reviewed-on: https://chromium-review.googlesource.com/499307
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
This will allow git cl format to handle histograms.xml being
split into several files, and also adds support for UKM.
Bug:699328
Change-Id: I2bfbd333b78b679f69d7cb298f5adaf02f6a4dec
Reviewed-on: https://chromium-review.googlesource.com/483979
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Steven Holte <holte@chromium.org>
This makes the library behave more like what we'd expect,
while still allowing certain function to specify that
returning 204 or 404 is expected/acceptable behavior.
Change-Id: If3ce5598d1603819ee97aaeab0072a9e786ed96d
Reviewed-on: https://chromium-review.googlesource.com/481043
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Gerrit sometimes returns a full response json object at
the same time as returning a non-200 status code. This
refactor makes it easier for calling code to request
access to that object and handle error cases on its own.
The original version of this commit had a bug where
ReadHttpResponse properly set the default value for
accept_statuses, but all calls which came through
ReadHttpJsonResponse were setting None instead.
Bug: 710028
Change-Id: I8cee435d8acd487fb777b3fd69b5e48e19d2e5a3
Reviewed-on: https://chromium-review.googlesource.com/481060
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
We want to assert that if add_owners_to is provided, change is also
provided.
Bug:
Change-Id: I0b9674dcd1cdd5e42a8ab92583570ecc02ba1dcd
Reviewed-on: https://chromium-review.googlesource.com/481040
Reviewed-by: Daniel Jacques <dnj@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
This reverts commit 6d7ab1bfe5.
Reason for revert: Stacktrace:
File "/s/depot_tools/gerrit_util.py", line 816, in GetAccountDetails
return ReadHttpJsonResponse(conn)
File "/s/depot_tools/gerrit_util.py", line 376, in ReadHttpJsonResponse
fh = ReadHttpResponse(conn, accept_statuses)
File "/s/depot_tools/gerrit_util.py", line 365, in ReadHttpResponse
if response.status not in accept_statuses:
TypeError: argument of type 'NoneType' is not iterable
Original change's description:
> Refactor ReadHttpResponse to be error-friendlier
>
> Gerrit sometimes returns a full response json object at
> the same time as returning a non-200 status code. This
> refactor makes it easier for calling code to request
> access to that object and handle error cases on its own.
>
> Bug: 710028
> Change-Id: Id1017d580d2fb843d5ca6287efcfed8775c52cd6
> Reviewed-on: https://chromium-review.googlesource.com/479450
> Commit-Queue: Aaron Gable <agable@chromium.org>
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
>
TBR=agable@chromium.org,tandrii@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: Ia9d9ce835e207a32e7cc8ee35c0cf40c823c7b78
Reviewed-on: https://chromium-review.googlesource.com/481059
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Gerrit sometimes returns a full response json object at
the same time as returning a non-200 status code. This
refactor makes it easier for calling code to request
access to that object and handle error cases on its own.
Bug: 710028
Change-Id: Id1017d580d2fb843d5ca6287efcfed8775c52cd6
Reviewed-on: https://chromium-review.googlesource.com/479450
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This will be useful for tools like the recipe autoroller to
semi-intelligently choose a collection of reviewers.
Bug:
Change-Id: I7a238127e126340c033fc8e80523ef2408ebf970
Reviewed-on: https://chromium-review.googlesource.com/479679
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
This CL propagates <diff_base> all the way to become parent commit
of the syntentic commit generated by squashing the current branch.
BUG=649846
R=agable@chromium.org
Change-Id: Ided7ebbb5c3a1114cac18adb62b3a9c27610018c
Reviewed-on: https://chromium-review.googlesource.com/475229
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
While working on fixing git-cl-status for gerrit, I realized
it would be really easy to bring the Rietveld version up to
parity and simplify it at the same time.
Bug: 706460
Change-Id: Icff32b532fa29f8869205111cd117176e0d34b8f
Reviewed-on: https://chromium-review.googlesource.com/470448
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
The loop over the messages didn't early-exit when it found
a non-CQ message, so it would continue looking until it
found any message by a non-owner. So all CLs with any comments
by anyone other than the uploader would be in 'reply' state
forever.
This CL fixes that, and also makes some other tweaks to
make the gerrit status code simpler and faster.
Bug: 706460
Change-Id: I5cc06962f5121fe042a315e5e2e205e556eb85da
Reviewed-on: https://chromium-review.googlesource.com/470586
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
* if --rietveld or --gerrit is given, parse only using appropriate
parser.
* if url has '$HOST-review' in the beginning, assume it's Gerrit
* if type of codereview is detected based on URL, then print this
information for the user.
This also applies to `git cl description` but message is logged instead,
because in '-d|--display' option git cl is supposed to print only description,
and some tooling likely relies on this :(
R=jochen@chromium.org
BUG=706406
Change-Id: I21c9355c5334fd71db27618cda11287f75168b59
Reviewed-on: https://chromium-review.googlesource.com/473186
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
This is temporary, and will be changed in subsequent CLs. For now,
it suffices to stop relying on pseudo-random dictionary order in tests
and prod.
R=jochen@chromium.org
BUG=706406
Change-Id: I26c467a28bc63b5f81d20fc222a2b6f0511c507f
Reviewed-on: https://chromium-review.googlesource.com/472750
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
PolyGerrit used to surround emails with invisible unicode char, which
resulted in reviewers not being found by Gerrit. It has since been
fixed (handled in Google internal bug b/34702620).
R=sergiyb@chromium.org
Change-Id: I43ade0ef23d88611081744eff57129a69b5176c8
Reviewed-on: https://chromium-review.googlesource.com/472808
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This reverts commit 0267fd2c29.
Reason for revert: seems to have changed some behavior as reported by SKIA.
Original change's description:
> git cl upload for Gerrit: use push options instead of refspec.
>
> This removes limitation of no special chars in patchset titles.
>
> BUG=chromium:663787,chromium:707963,gerrit:5184
> R=sergiyb@chromium.org,agable@chromium.org
> TEST=uploaded this CL using depot_tools with this patch :)
>
> Change-Id: I5d684d0a0aa286a45ff99cca6d57aefa8436cd0f
> Reviewed-on: https://chromium-review.googlesource.com/468926
> Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Sergiy Byelozyorov <sergiyb@google.com>
>
TBR=agable@chromium.org,sergiyb@google.com,tandrii@chromium.org,sergiyb@chromium.org,borenet@chromium.org,chromium-reviews@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:663787,chromium:707963,gerrit:5184
Change-Id: I3306091b14b97a200150389d0480b69120af8c61
Reviewed-on: https://chromium-review.googlesource.com/469006
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
This removes limitation of no special chars in patchset titles.
BUG=chromium:663787,chromium:707963,gerrit:5184
R=sergiyb@chromium.org,agable@chromium.org
TEST=uploaded this CL using depot_tools with this patch :)
Change-Id: I5d684d0a0aa286a45ff99cca6d57aefa8436cd0f
Reviewed-on: https://chromium-review.googlesource.com/468926
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Sergiy Byelozyorov <sergiyb@google.com>
This allows for having some global comments such as timezones or
long-term unavailability.
The comments go into build/OWNERS.status (that way, they should be
available in all repos that map in build/ for the gn config files).
The local can be overwritten in codereview.settings.
The format is
email: status
Comments (starting with #) are allowed in that file, but they're ignored.
BUG=694222
R=dpranke@chromium.org
Change-Id: I49f58be87497d1ccaaa74f0a2f3d373403be44e7
Reviewed-on: https://chromium-review.googlesource.com/459542
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
This is a hack to support the most common git workflow: keeping
a local branch named 'master' which is a replica of 'origin/master'
with no local changes, and basing all feature branches off of
that.
Test:
* created master branch, created feature branch, uploaded CL correctly
* created master branch, landed change on master branch, created
feature branch, uploaded CL, CL contains change on master too
* created non-master branch, created feature branch, failed to
upload CL
Bug: 682104
Change-Id: I8481b787e6dcab162d2846c07f1ddad950f491e0
Reviewed-on: https://chromium-review.googlesource.com/464107
Reviewed-by: Andrew Bonventre <andybons@google.com>
Commit-Queue: Aaron Gable <agable@chromium.org>
This is needed for making PostUploadHooks that don't have to re-invent
the git footers library.
Bug:
Change-Id: I0a0ccf3dffd25152c2c273487ddbd9b279d80678
Reviewed-on: https://chromium-review.googlesource.com/461729
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
The issue is with directory separators:
"Check that c:\src\chromium\src or
c:\src\chromium\src\tools/metrics/histograms/pretty_print.py
exist and have execution permission."
BUG=679514
Change-Id: I88a57783ca3bf8aa5af4d83fb34901c2d2385fe9
Reviewed-on: https://chromium-review.googlesource.com/457800
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Pierre-Antoine Manzagol <manzagop@chromium.org>
Note that because it is now a gerrit footer, it both appears in the same block
as the Change-Id footer (no blank line between them), and isn't guaranteed to be
above the Change-Id footer. This doesn't matter during "git cl upload", when
a Change-Id hasn't been allocated yet, but will show up during "git cl
description".
Bug: 681184
Change-Id: I2ab6fc13be8e992709618a666012410b1a7c02de
Reviewed-on: https://chromium-review.googlesource.com/446660
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
These methods are used to produce report in the next CL.
BUG=689543
Change-Id: I71b2705ac8b046103b4982d47f7ec97f8ef7818b
Reviewed-on: https://chromium-review.googlesource.com/455838
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
If the commit or cherry-pick command fails, git automatically drops
the user into a state where they can try to manually fix it. That's
great. But with the old call order, it would also leave the metadata
unset. This sets the issue and patchset number before doing the
potentially-failing heavy lifting, so that the branch will be in a
consistent state after the user finishes fixing up the failed command.
BUG=701130
Change-Id: I792b9fb9e61ba62626c19aa1837d21f8cd8f594e
Reviewed-on: https://chromium-review.googlesource.com/456039
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Commit-Queue: Aaron Gable <agable@chromium.org>
Aaron Gable
Andrii Shyshkalov
Steven Holte
Robert Iannucci
Jochen Eisinger
Pierre-Antoine Manzagol