[Announcement] Gerrit is now open for business!

[Aaron Gable]

Hi Chromium and Blink devs,

tl;dr: Use gerrit in chromium/src with git cl upload --gerrit! File bugs here! Let us know what you think!

As we've announced before, Chromium is in the process of changing over from using Rietveld for code review to using Gerrit. As of today, Gerrit is ready for the full force of your usage and testing. We'd like to invite all Chromium contributors to dogfood Gerrit for your code reviews.

Here are some things we've improved thanks to early dogfood feedback:

• Re-authentication after credentials expire is now easy and nearly seamless

• We no longer send separate emails to each person on the CC line

• Image diffs are now just as fast as text diffs

We also have some exciting stuff coming up soon:

• Themes for different hosts (e.g. external and internal), so you can tell them apart at a glance

• A Work-In-Progress mode that won't send any emails until you Publish your CL

• An easy way to reland CLs which have been reverted

Whenever you're uploading a code review against chromium/src.git, please consider uploading the change to Gerrit instead of Rietveld.

To upload a change to Gerrit instead of Rietveld, just run

   git cl upload --gerrit

when uploading the change for the first time. Additional patchsets after the first will automatically upload to the same review. If you would like all of your Chromium reviews to go to Gerrit without having to pass the flag, simply run

   git config --local gerrit.host true

in your local repository checkout. If you have done the above and need a temporary escape hatch, you can upload with git cl upload --rietveld, and please file a bug or send feedback about why you needed to switch back.

When using Gerrit, you should get the new UI (PolyGerrit) by default. If you don't (because you have used Gerrit in the past and expressed a UI preference then), please click the "New UI" link in the footer or follow this link to use the new one instead.

This FAQ should hopefully address any questions you might have. The settings page also has a lot of things you can tweak if they aren't to your liking. If you run into any issues interacting with your CLs from the command line or landing them via the commit queue, please file a bug here. If you run into any issues using the PolyGerrit web application, please file a bug at here.

We really value your feedback on Gerrit and want to hear about your experience using it so that we can continue improving it and make this transition as smooth as possible. We look forward to hearing from you!

Thanks,

Aaron

P.S. Yes, this does mean that we'll be using both Gerrit and Rietveld side-by-side for the near future. The more helpful the feedback we receive is, the quicker we'll be able to switch away from Rietveld entirely.

[Primiano Tucci]

Excited to see progress on this!

I just have one question: does this mean that whether a CL comes from gerrit depends purely on the choice of the author now?

If so, does it mean that, as a reviewer, I have to watch now both codereview systems?

--
You received this message because you are subscribed to the Google Groups "infra-announce" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-announc...@chromium.org.
To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.
To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

[Nico Weber]

On Thu, May 11, 2017 at 9:43 AM, Primiano Tucci <prim...@chromium.org> wrote:

Excited to see progress on this!

I just have one question: does this mean that whether a CL comes from gerrit depends purely on the choice of the author now?

If so, does it mean that, as a reviewer, I have to watch now both codereview systems?

I think this has been true for a while already (people could opt in to gerrit before – at least I've been getting gerrit reviews from dcheng for chromium things for a while).

 

To unsubscribe from this group and stop receiving emails from it, send an email to infra-announce+unsubscribe@chromium.org.

To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

[Andrew Grieve]

Some feedback from trying it out this past week:

1. If you try to reuse the same upload message as a previous patch (I often just type "rebase"), upload fails with a 500 error.

2. Sometimes even with a unique message, it fails, but works upon retry. E.g.:

To https://chromium.googlesource.com/chromium/src.git
 ! [remote rejected]           ac39616cdd36d2db4fb5b3dcc7551e1fb1424891 -> refs/for/refs/heads/master%m=rebase2,notify=NONE (internal server error: Error inserting change/patchset)

But another run of "git cl upload" succeeded. This has happened to me a few times this week.

3. Uploads are not as snappy as with Rietveld. I'm finding they take between 1 and 2 minutes to complete.

4. Despite these minor issues, I've been overall quite happy with it!

[Aaron Gable]

I'll respond to all of you in one go :)

On Thu, May 11, 2017 at 7:12 AM Andrew Grieve <agr...@chromium.org> wrote:

Some feedback from trying it out this past week:

1. If you try to reuse the same upload message as a previous patch (I often just type "rebase"), upload fails with a 500 error.

That's very odd, and not a behavior I've been able to reproduce (e.g. both patchsets here have the exact same title). Please file a bug here with reproduction steps.

 

2. Sometimes even with a unique message, it fails, but works upon retry. E.g.:

To https://chromium.googlesource.com/chromium/src.git
 ! [remote rejected]           ac39616cdd36d2db4fb5b3dcc7551e1fb1424891 -> refs/for/refs/heads/master%m=rebase2,notify=NONE (internal server error: Error inserting change/patchset)

But another run of "git cl upload" succeeded. This has happened to me a few times this week.

Yes, we're aware of this. There have been two minor gerrit outages in the past couple weeks which led to increased levels of 500s on write operations, i.e. failed pushes.

 

3. Uploads are not as snappy as with Rietveld. I'm finding they take between 1 and 2 minutes to complete.

Woah, 1 to 2 minutes is unacceptable. I've never seen that myself, but we have received at least one other complaint about slowness. Can you fill in on that bug some details? In particular, is it slow for certain kinds of patchsets (e.g. ones with a very large diff, or ones where your local branch has a lot of commits on it)? At what point during the upload is it slow (e.g. after printing the summary of the diff it will upload, or during Processing Changes, or some other time)?

 

4. Despite these minor issues, I've been overall quite happy with it!

Yay!

 

On Thu, May 11, 2017 at 9:45 AM, Nico Weber <tha...@chromium.org> wrote:

On Thu, May 11, 2017 at 9:43 AM, Primiano Tucci <prim...@chromium.org> wrote:

Excited to see progress on this!

I just have one question: does this mean that whether a CL comes from gerrit depends purely on the choice of the author now?

If so, does it mean that, as a reviewer, I have to watch now both codereview systems?

I think this has been true for a while already (people could opt in to gerrit before – at least I've been getting gerrit reviews from dcheng for chromium things for a while).

Yes, as noted in the postscript of the PSA, this does mean that we'll be using both systems side-by-side for a little while. (And as Nico points out, we already have been, although at a smaller scale.) That's why we need feedback from y'all to be as helpful as possible, and we need to act on it as fast as possible, so that we can get into a Gerrit-only state quickly and smoothly.

Thanks again, everybody!

Aaron

 

 

To unsubscribe from this group and stop receiving emails from it, send an email to infra-announc...@chromium.org.

To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

[Ken Rockot]

Just want to chime in as someone who's been dogfooding PolyGerrit for a few weeks now. There are a few mildly annoying issues (soooo many emails) which already have bugs filed, but I'm really quite happy with it so far and have absolutely no desire to return to Rietveld. The code review UI is objectively more powerful and the learning curve from Rietveld is tiny. IMHO more people should start using the new tool sooner rather than later.

Thanks for the hard work so far!

To unsubscribe from this group and stop receiving emails from it, send an email to infra-announce+unsubscribe@chromium.org.

To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

[Morten Stenshorne]

On Thu, May 11 2017 at 21:32, Ken Rockot <roc...@chromium.org> wrote:

> Just want to chime in as someone who's been dogfooding PolyGerrit for a few
> weeks now. There are a few mildly annoying issues (soooo many emails

And HUGE! (which apparently also makes them heavy to load, at least with
my client)

--
---- Morten Stenshorne, developer, Opera Software ----
------------------ http://www.opera.com/ -------------

[Paweł Hajdan, Jr.]

+infra-dev (IMO useful to keep in the loop)

On Thu, May 11, 2017 at 9:22 PM, Aaron Gable <aga...@chromium.org> wrote:

On Thu, May 11, 2017 at 7:12 AM Andrew Grieve <agr...@chromium.org> wrote:

1. If you try to reuse the same upload message as a previous patch (I often just type "rebase"), upload fails with a 500 error.

That's very odd, and not a behavior I've been able to reproduce (e.g. both patchsets here have the exact same title). Please file a bug here with reproduction steps.

Intermittent bugs might be hard to catch/debug this way.

Do we have way to inspect the server-side logs once a user reports an error like this? Say they give us a CL ID, and then we just look up any HTTP 500s, and make sure any relevant details get included in the logs.

This technique proved very powerful when eradicating issues with CQ, both the python daemon itself, and any integration issues in related components.

 

2. Sometimes even with a unique message, it fails, but works upon retry. E.g.:

To https://chromium.googlesource.com/chromium/src.git
 ! [remote rejected]           ac39616cdd36d2db4fb5b3dcc7551e1fb1424891 -> refs/for/refs/heads/master%m=rebase2,notify=NONE (internal server error: Error inserting change/patchset)

But another run of "git cl upload" succeeded. This has happened to me a few times this week.

Yes, we're aware of this. There have been two minor gerrit outages in the past couple weeks which led to increased levels of 500s on write operations, i.e. failed pushes.

We may already have it, but just checking: do we have automatic exponential retry for this in git-cl?

I found that for many workflows, it's nice to run a fire-and-forget command: even if it's slow, I can switch to another terminal or email, and do something useful without need to manually monitor it.

 

3. Uploads are not as snappy as with Rietveld. I'm finding they take between 1 and 2 minutes to complete.

Woah, 1 to 2 minutes is unacceptable. I've never seen that myself, but we have received at least one other complaint about slowness. Can you fill in on that bug some details? In particular, is it slow for certain kinds of patchsets (e.g. ones with a very large diff, or ones where your local branch has a lot of commits on it)? At what point during the upload is it slow (e.g. after printing the summary of the diff it will upload, or during Processing Changes, or some other time)?

Also pushing a little bit: what's out ability to instrument things here, both server side and in git-cl?

The more we info we can get ourselves as opposed to requiring our users to repro intermittent issues, the faster we can fix issues like this. I'm speaking from experience solving intermittent issues in other systems.

Hopefully these ideas are useful, and you may have considered them. I was wondering if you'd consider them helpful, so that we can make the Chromium Gerrit launch successful. Looks like it's on the right track - it's great to see responsiveness to issues raised in this thread.

Thanks!

Paweł

 

To unsubscribe from this group and stop receiving emails from it, send an email to infra-announce+unsubscribe@chromium.org.

To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

[Jean-François Geyelin]

Note that

git config --local gerrit.host false

does not disable gerrit.

You have to remove the option altogether from .git/config

[Bo Liu]

For those like me who miss the rietveld's +owner/+reviewer feature in emails, gerrit does have an equivalent feature: hidden footers in the email.

[Aaron Gable]

On Fri, May 12, 2017 at 10:10 AM Bo Liu <bo...@chromium.org> wrote:

For those like me who miss the rietveld's +owner/+reviewer feature in emails, gerrit does have an equivalent feature: hidden footers in the email.

Thanks for pointing that out! Yes, Gerrit emails contain lots of hidden footers. I use them to filter emails into separate labels for changes I uploaded, changes I'm a reviewer on, and changes I'm CC'd on with filters like "includes 'Gerrit-Reviewer: Aaron Gable'". We have a bug open to document them better, and I'll be adding a section to the FAQ for this.

 

On Fri, May 12, 2017 at 8:51 AM, Jean-François Geyelin <j...@chromium.org> wrote:

Note that

git config --local gerrit.host false

does not disable gerrit.

You have to remove the option altogether from .git/config

Ooh, thanks for pointing that out. That's not working as intended, and I have a fix here. Once that lands, setting 'git config --local gerrit.host false' (or any value other than 'true') should work.

Thanks again,

Aaron

P.S. Also keep filing bugs here!

[Vincent Scheib]

"[ ]" keys in gerrit don't match "j k" keys

j k keys move to next/previous. Also true in gmail and other apps.

[ ] keys in gerrit are flipped, and go to previous/next instead. 

This is inconsistent in gerrit, but also with gmail.

Should this be fixed? This dilemma may reach the levels of consternation Chrome's Backspace key caused, where some wanted it to navigate, and others hated loosing data. Thankfully, the gerrit user community is easier to poll. Have at it, express you opinion!

Poll: https://docs.google.com/forms/d/e/1FAIpQLScS9F6bgfJOjdrl-fn9n_trg0pne91DSokM8sczYi0SUtbJvA/viewform?usp=sf_link

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2c_A5ZR-cbfZuN7ysaB7gBxQCy%3DRif%2B3iK_By5OKM-w4w%40mail.gmail.com.

[Mike Frysinger]

chromium+blink != gerrit community

i'm not following what you're saying though.  the j/k and [/] behavior looks fine to me in gerrit.

j = move down one line

k = move up one line

[ = move to the "left" (previous file)

] = move to the "right" (next file)

-mike

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAK-EfXk510V_sRd2GVAYT-vUSmuMgWLdvQOUW_bd4HV6hkQ4hw%40mail.gmail.com.

[Vincent Scheib]

I've also sent the poll to https://groups.google.com/forum/#!aboutgroup/repo-discuss (though it needs to get through moderation)

chromium+blink are plenty relevant to the gerrit community, and are much less likely to be members of repo-discuss.

jk move to next,previous of a both lines and files with comments (shift modifier).

[] move to previous,next files.

[Mike Frysinger]

to put it into context: you're talking about changing behavior that Gerrit has had for at least 3.5 years.

-mike

[Vincent Scheib]

Yes, I appreciate the context and that's why I compared it to Chrome's change of the Backspace key. It may not be as big a deal as no headphone jack, though. That.. that took courage.

[Mike Frysinger]

Chromium OS has been using Gerrit for like 6 years at this point, and it's not on any of those three lists

i can't speak to the coverage repo-discuss gives as gerrit-as-a-project vs repo-as-a-tool

-mike

[Aaron Gable]

repo-discuss is the primary mailing list for open-source discussion and development of Gerrit. So it is basically the right place to reach out to the wider Gerrit community and get their thoughts on changing their keyboard shortcuts.

However, this thread isn't really the right place for this discussion. Rietveld never had [/] keyboard shortcuts, so there is no call to change what they do in PolyGerrit in order to accommodate folks migrating from Rietveld. Gerrit has had [/] keyboard shortcuts for a very long time, and PolyGerrit matches that behavior, so there is no call to change what they do in PolyGerrit to accommodate folks migrating from the old Gerrit UI. So as much as I think the question of whether or not the keyboard shortcuts should match GMail's behavior is one worth asking, the migration of Chromium to Gerrit is not the best time or place to do so.

Thanks,

Aaron

[dan...@chromium.org]

On Fri, May 12, 2017 at 2:05 PM, Aaron Gable <aga...@chromium.org> wrote:

On Fri, May 12, 2017 at 10:10 AM Bo Liu <bo...@chromium.org> wrote:

For those like me who miss the rietveld's +owner/+reviewer feature in emails, gerrit does have an equivalent feature: hidden footers in the email.

Thanks for pointing that out! Yes, Gerrit emails contain lots of hidden footers. I use them to filter emails into separate labels for changes I uploaded, changes I'm a reviewer on, and changes I'm CC'd on with filters like "includes 'Gerrit-Reviewer: Aaron Gable'". We have a bug open to document them better, and I'll be adding a section to the FAQ for this.

How do people write filters for these? I made a filter for Has words "Gerrit-Reviewer: danakj", but this picks up other emails that contain each of those words but not together, such as

Gerrit-Owner: danakj

Gerrit-Reviewer: someone else

Is there gmail magic to prevent this incorrect matching?

 

 

On Fri, May 12, 2017 at 8:51 AM, Jean-François Geyelin <j...@chromium.org> wrote:

Note that

git config --local gerrit.host false

does not disable gerrit.

You have to remove the option altogether from .git/config

Ooh, thanks for pointing that out. That's not working as intended, and I have a fix here. Once that lands, setting 'git config --local gerrit.host false' (or any value other than 'true') should work.

Thanks again,

Aaron

P.S. Also keep filing bugs here!

--

[Bo Liu]

On Mon, May 15, 2017 at 9:00 AM, <dan...@chromium.org> wrote:

On Fri, May 12, 2017 at 2:05 PM, Aaron Gable <aga...@chromium.org> wrote:

On Fri, May 12, 2017 at 10:10 AM Bo Liu <bo...@chromium.org> wrote:

For those like me who miss the rietveld's +owner/+reviewer feature in emails, gerrit does have an equivalent feature: hidden footers in the email.

Thanks for pointing that out! Yes, Gerrit emails contain lots of hidden footers. I use them to filter emails into separate labels for changes I uploaded, changes I'm a reviewer on, and changes I'm CC'd on with filters like "includes 'Gerrit-Reviewer: Aaron Gable'". We have a bug open to document them better, and I'll be adding a section to the FAQ for this.

How do people write filters for these? I made a filter for Has words "Gerrit-Reviewer: danakj", but this picks up other emails that contain each of those words but not together, such as

Gerrit-Owner: danakj

Gerrit-Reviewer: someone else

Is there gmail magic to prevent this incorrect matching?

My filters. For CLs I wrote:

Matches: "Gerrit-Owner: Bo Liu <bo...@chromium.org>"

For CLs for me to review:

Matches: -"Gerrit-Owner: Bo Liu <bo...@chromium.org>" "Gerrit-Reviewer: Bo Liu <bo...@chromium.org>"

So quotation marks works for exact matches. And "-" for excludes this exact match.

 

 

On Fri, May 12, 2017 at 8:51 AM, Jean-François Geyelin <j...@chromium.org> wrote:

Note that

git config --local gerrit.host false

does not disable gerrit.

You have to remove the option altogether from .git/config

Ooh, thanks for pointing that out. That's not working as intended, and I have a fix here. Once that lands, setting 'git config --local gerrit.host false' (or any value other than 'true') should work.

Thanks again,

Aaron

P.S. Also keep filing bugs here!

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2c_A5ZR-cbfZuN7ysaB7gBxQCy%3DRif%2B3iK_By5OKM-w4w%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHtyhaQZ5ShrcbWrxFuUhtTBS5tgz4_j1cwq8UxCSW0_wquXmg%40mail.gmail.com.

[Aaron Gable]

@dominick: Yeah, while PRESUBMIT can make uploads incredibly slow, there is a separate problem with the upload itself in something like ~0.5% of uploads. We're tracking it down.

@danakj: Yep, what Bo said is exactly what I do. Quotes for exact matches work well. I've also documented the rules in the FAQ.

[Christian Biesinger]

I have a question about how things like BUG= and R= work:

The default template seems to use "Bug: 123", so I assume that will
work to update the right bug.

But how does that work for R=? Should I keep using
"R=f...@chromium.org" or is it something like "R: f...@chromium.org"?

Thanks!
Christian

[Aaron Gable]

Good question! For now, both work. As we move into a gerrit-only world (which has sane metadata, unlike Rietveld), we'll stop using R= entirely. If you set it in your commit description, we'll strip it out and set the reviewers directly on the CL metadata instead. In the mean time, ALL_CAPS= and Title-Case: tags/footers can coexist peacefully.

[Jeremy Roman]

What about the other miscellaneous annotations, like CQ_INCLUDE_TRYBOTS, NOTRY, TBR, NOPRESUBMIT, NOTREECHECKS, and so on? Do these all have a Title-Case counterpart, or should some still be in ALL_CAPS= format?

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2cqV_u9vth0vk%2Bobc9ybOSZ2S%3DyxKncy_G61P6-UCsVEw%40mail.gmail.com.

[Thiago Farina]

On Wed, May 17, 2017 at 5:44 PM, Aaron Gable <aga...@chromium.org> wrote:

Good question! For now, both work. As we move into a gerrit-only world (which has sane metadata, unlike Rietveld), we'll stop using R= entirely. If you set it in your commit description, we'll strip it out and set the reviewers directly on the CL metadata instead. In the mean time, ALL_CAPS= and Title-Case: tags/footers can coexist peacefully.

From my experience, I would recommend not adding R= to the commit message. If you do,

then Gerrit will parse it and add the reviewer automatically without sending an email asking him

to review your change.

--

Thiago Farina

[Aaron Gable]

The CQ respects git-footer style versions of all of those:

Cq-Include-Trybots, Tbr, No-Try, No-Presubmit, and No-Tree-Checks.

In addition, we'll be working on moving TBR=/Tbr: to pure metadata with no line in the commit message.

[Aaron Gable]

Yes, that is correct, which is the same as Rietveld's behavior.

Right now, git-cl-upload behaves the same when uploading to both Gerrit and Rietveld: it takes the combination of all reviewers specified on R= lines in the description and "-r" flags on the command line, writes it to the commit description, and also sets it as metadata. Whether it sends mail or not is completely dependent on whether you passed the "--send-mail/-s" flag at the same time. In the near future, we'll be improving this in two ways: taking advantage of the new Work-In-Progress gerrit state so that the need to publish the CL is more apparent to the uploader, and removing the R= from the commit description and using only the metadata.

[Christian Biesinger]

I'm probably fighting a losing battle here but I'd rather you keep the
R= in the commit description. I find it valuable to have the
reviewer's name in the git log.

(Thanks for confirming that R= with --send-mail do the right thing!)

Christian

[Mike Frysinger]

with gerrit, people who have reviewed the file (i.e. given it a Code-Review+1 or Code-Review+2) will show up in the commit message automatically

as an example:

https://chromium.googlesource.com/chromiumos/chromite/+/643c1dc25efb74f17afe06affc350366521ce019

those extended tags weren't authored by anyone

-mike

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
    http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAPTJ0XFQ4nRO2Ydd%2BD2vXmfKZGkqUq0jGrm20YcdwQ9env0_Kg%40mail.gmail.com.

[Thiago Farina]

On Wed, May 17, 2017 at 6:23 PM, Christian Biesinger <cbies...@chromium.org> wrote:

I'm probably fighting a losing battle here but I'd rather you keep the
R= in the commit description. I find it valuable to have the
reviewer's name in the git log.

(Thanks for confirming that R= with --send-mail do the right thing!)

I used R= pretty much in all my Rietveld CLs, but when landing through Gerrit,

it adds Reviewed-by: line automatically for you, for whom ever approved your

patch, which is great and superior to Rietveld's R=.

--

Thiago Farina

[Christian Biesinger]

Oh that's neat! I'm happy to see that!

Christian

[Aaron Gable]

Yep, that's exactly why we're doing it. We're removing the R= because it only reflects the invited reviewers, not the actual reviewers. The Reviewed-By line reflects the folks who actually chimed in and approved the CL. Similarly, since Gerrit has a well-defined sense of "self-approval" (voting Code-Review+1 on your own CL), we're going to replace TBR= with automatically adding "TBRed-To: " lines for the folks who were reviewers on the CL but hadn't actually reviewed when you submit with only self-approval. That way the commit message indicates the actual state of the review, not whatever the uploader chose to write when they first created the CL.

[Christian Biesinger]

Your emails led me to believe that "R: f...@chromium.org" should have
worked too but it didn't, only "R=..." works. Is that intentional? I
get this with "R:":

Error after CL description prompt -- saving description to
/usr/local/google/home/cbiesinger/.git_cl_description_backup

Must specify reviewers to send email.

[Aaron Gable]

Whoops, sorry, that's right, we haven't made "R:" work with git-cl because we plan to remove R= entirely, rather than support it as a git-footer as well. Sorry for the confusion.

[Will Harris]

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

I don't see a +2 option on reviewing someone else's CLs, but I see it mentioned in the above thread, is +2 only for certain CLs, or for authors of CLs?

Should a patch author CQ +1 or CQ +2 - and if so, when should this be done, i.e. does CQ +2 mean autocommit? Is there an autocommit?

What is the difference between doing 'reply' and CQ +1 and clicking the 'CQ Dry Run' at the top right?

What exactly does clicking 'Submit to CQ' button in the top right do?

What exactly does clicking 'CQ Dry Run' do?

Is this doing a +1 or +2 on the CL? Why is there even a +1 or +2 CQ option in the 'reply' menu, if these buttons also do this? (or maybe they don't).

What does the 'rebase' button do? If I don't push this button and the code underneath my CL changes, will the CL fail to commit? Does the rebase button affect anything in my local workspace? If I do not rebase, are my trybot tests running at the revision I uploaded the CL at, or at head revision? What happens if I rebase my CL locally and upload a new PS, do I have to click the rebase button also?

Thanks,

Will

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2euQUb4rfBurTFEApYhwZSQ%3D059cw91MTh9Zg6rvwrQrQ%40mail.gmail.com.

[Thiago Farina]

On Tue, May 23, 2017 at 10:04 PM, Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

Code-Review +1 means Looks good but someone else should approve.

Code-Review +2 means Looks good, approve.

 

I don't see a +2 option on reviewing someone else's CLs, but I see it mentioned in the above thread, is +2 only for certain CLs, or for authors of CLs?

Should a patch author CQ +1 or CQ +2 - and if so, when should this be done, i.e. does CQ +2 mean autocommit? Is there an autocommit?

CQ +1 means CQ Dry Run (i.e., trybots).

CQ +2 means Submit to CQ (i.e., land this patch).

--

Thiago Farina

[Thiago Farina]

Will,

Basically the workflow will be something very rouchly like this:

1- Author: git cl upload --gerrit

2- Author: CQ +1

3- Wait for review

4- Reviewer: Code-Review+2

5- Author: CQ +2

6- Patch merged 

--

Thiago Farina

[Will Harris]

On Wed, May 24, 2017 at 11:10 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:04 PM, Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

Code-Review +1 means Looks good but someone else should approve.

Code-Review +2 means Looks good, approve.

If person A does a +1 and person B also does a +1 then is this a +2? Does it matter of person A or Person B are or are not owners of the directories containing the files in the CL? Does the author of the CL have to be one of the people doing a +1?

[Will Harris]

On Wed, May 24, 2017 at 11:13 AM, Thiago Farina <tfa...@chromium.org> wrote:

Will,

Basically the workflow will be something very rouchly like this:

1- Author: git cl upload --gerrit

2- Author: CQ +1

3- Wait for review

Should I not be reviewing CLs unless they have a CQ +1 from the author?

 

4- Reviewer: Code-Review+2

I only had the option to +1 the last CL I reviewed. Did I do something wrong? 

[Sunny Sachanandani]

Chromium doesn't use +2 for Code-Review. For Chromium, +1 = lgtm and -1 = not lgtm and that's it. The CQ field is separate from Code-Review and there +1 = dry-run (git cl try) and +2 = submit to CQ (git cl set-commit).

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BFVm4sZ9_SM%2B_UG-tXjb%3D8qRSR7EnbG6hTYe1hL0rmDWgpczQ%40mail.gmail.com.

[Thiago Farina]

On Tue, May 23, 2017 at 10:14 PM, Will Harris <w...@chromium.org> wrote:

On Wed, May 24, 2017 at 11:10 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:04 PM, Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

Code-Review +1 means Looks good but someone else should approve.

Code-Review +2 means Looks good, approve.

If person A does a +1 and person B also does a +1 then is this a +2?

No, I don't think it sums up like this.

The way I see, +1 is usually from your team mate reviewing your patch, the +2 will come from the owner

of the code you are changing.

Does it matter of person A or Person B are or are not owners of the directories containing the files in the CL? Does the author of the CL have to be one of the people doing a +1?

No, you usually won't Code-Review +1 your own CLs, that is sign that will be given by whoever is reviewing

your change.

--

Thiago Farina

[Thiago Farina]

On Tue, May 23, 2017 at 10:15 PM, Will Harris <w...@chromium.org> wrote:

On Wed, May 24, 2017 at 11:13 AM, Thiago Farina <tfa...@chromium.org> wrote:

Will,

Basically the workflow will be something very rouchly like this:

1- Author: git cl upload --gerrit

2- Author: CQ +1

3- Wait for review

Should I not be reviewing CLs unless they have a CQ +1 from the author?

 

If you received an e-mail with a message like this: "Random Hacker would like Will Harris to review this change.",

then I think it is perfectly find to start reviewing it. Otherwise, I would just wait until the author think it is ready.

But, in general, you don't need to wait for the author to CQ +1 his patch.

 

4- Reviewer: Code-Review+2

I only had the option to +1 the last CL I reviewed. Did I do something wrong? 

Probably not.

As Sunny said below it is:

CQ +1 = dry run (git cl try)

CQ +2 = submit to CQ (git cl set-commit)

Code-Review -1 = NOT LGTM

Code-Review +1 = LGTM

--

Thiago Farina

[Thiago Farina]

On Tue, May 23, 2017 at 10:38 PM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:15 PM, Will Harris <w...@chromium.org> wrote:

On Wed, May 24, 2017 at 11:13 AM, Thiago Farina <tfa...@chromium.org> wrote:

Will,

Basically the workflow will be something very rouchly like this:

1- Author: git cl upload --gerrit

2- Author: CQ +1

3- Wait for review

Should I not be reviewing CLs unless they have a CQ +1 from the author?

 

If you received an e-mail with a message like this: "Random Hacker would like Will Harris to review this change.",

then I think it is perfectly find to start reviewing it. Otherwise, I would just wait until the author think it is ready.

s/find/fine 

--

Thiago Farina

[Aaron Gable]

Thanks for the questions! There are no dumb questions, just bad docs. Answers inline.

On Tue, May 23, 2017 at 6:04 PM Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Different projects have different label configurations. For example, ChromeOS (which has been using Gerrit for a while) uses both CR+1 and CR+2. The former means "this looks good, but I don't want to or don't have enough authority to approve", while the latter means "LGTM, ship it, fully approved". Chromium and other projects coming from Rietveld (skia, v8, webrtc, infra, etc) don't use CR+2 at all. Since Rietveld only had a binary "LGTM or not", we only use CR-1 (not lgtm) through CR+1 (lgtm).

 

Does it take Code Review +1 or +2 to approve a CL?

As mentioned above, in ChromeOS, it requires CR+2; in Chromium it requires CR+1. We are considering unifying these in the future, but decided that switching Chromium devs both from Rietveld to Gerrit and from a binary system to a 5-state system at the same time was too much.

 

I don't see a +2 option on reviewing someone else's CLs, but I see it mentioned in the above thread, is +2 only for certain CLs, or for authors of CLs?

The above thread doesn't mention CR+2, I don't believe. (It does mention CQ+2, which I'll cover below.) Anyway, the answer here is the same: it depends on the project. It also depends on your permissions; for example you might not have permission to set CR+2 in some of the ChromeOS repos that do use that value. Similarly, non-committers (i.e. new open-source contributors, interns, etc) don't have permission to set CR+2 in chromium/src.git.

 

Should a patch author CQ +1 or CQ +2 - and if so, when should this be done, i.e. does CQ +2 mean autocommit? Is there an autocommit?

Mouse over the labels and you'll see descriptions of what they mean. In particular, CQ+1 means "Dry run" while CQ+2 means "Commit". These behave the same as the commit queue on Rietveld did: Dry run results in the CQ running all the compiles/tests but not then committing, while Commit results in the CQ actually trying to land your patch. Just like with Rietveld, there is no autocommit.

 

What is the difference between doing 'reply' and CQ +1 and clicking the 'CQ Dry Run' at the top right?

Clicking "Reply" lets you also write a message. The 'CQ Dry Run' button is just for convenience. You can verify this for yourself by inspecting the messages (at the bottom of the page) which are generated when you take these actions.

 

What exactly does clicking 'Submit to CQ' button in the top right do?

It sets CQ+2, just like 'CQ Dry Run' sets CQ+2. You can see this by looking at the message that clicking the button generates.

 

What exactly does clicking 'CQ Dry Run' do?

See above, it sets CQ+1, which starts a CQ dry run, which does the exact same thing as it did on Rietveld.

 

Is this doing a +1 or +2 on the CL? Why is there even a +1 or +2 CQ option in the 'reply' menu, if these buttons also do this? (or maybe they don't).

Because the +1/+2 semantics are how Gerrit fundamentally works on the inside. It would be far too much work to hide them from the user entirely, so we don't. We simply offer tooltips and convenience buttons to explain the situation.

 

What does the 'rebase' button do?

It rebases your commit, just like if you ran "git rebase" in your local checkout. When you click it, it offers a selection of different sensible targets to rebase on top of (usually the most recent commit on 'master').

 

If I don't push this button and the code underneath my CL changes, will the CL fail to commit?

Only if there are merge conflicts. Remember that Rietveld was always dealing with true patches (textual diffs), which meant that the CQ and the Trybots were implicitly rebasing your patchset every time they fetched origin/master and then applied your patch. If your patch didn't apply, the bot and cq would fail. With Gerrit, it behaves the same, but using the actual git 'rebase' action instead of simply applying patches.

 

Does the rebase button affect anything in my local workspace?

No, that requires magic beyond our abilities at the moment.

 

If I do not rebase, are my trybot tests running at the revision I uploaded the CL at, or at head revision?

Just like with Rietveld, the trybots will always try your change against the latest version of master. They download your commit from Gerrit, then rebase it on top of origin/master to get the closest possible approximation of what the repo will look like after your change lands.

 

What happens if I rebase my CL locally and upload a new PS, do I have to click the rebase button also?

No, uploading a new patchset always just uploads whatever you have locally. The rebase button is a simple convenience so you don't have to do anything locally if you don't want to.

 

Thanks,

Will

Thanks again for the questions. I hope that the twelve emails which came in while I was writing this don't make it entirely moot :)

Aaron 

[Mike Frysinger]

On Tue, May 23, 2017 at 9:33 PM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:14 PM, Will Harris <w...@chromium.org> wrote:

On Wed, May 24, 2017 at 11:10 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:04 PM, Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

Code-Review +1 means Looks good but someone else should approve.

Code-Review +2 means Looks good, approve.

If person A does a +1 and person B also does a +1 then is this a +2?

No, I don't think it sums up like this.

correct -- don't look at these as any sort of summing operation.  they could be called Code-Review-Foo and Code-Review-Bar and have the same relationship.

The way I see, +1 is usually from your team mate reviewing your patch, the +2 will come from the owner

of the code you are changing.

that's one approach when OWNERS are available.  if you mouse over each label, it gives you a description of the meaning.

CR-2 = Do not submit

CR-1 = I would prefer you didn't submit this

CR-0 = No opinion

CR+1 = Looks good to me, but someone else must approve

CR+2 = Looks good to me, approved

Does it matter of person A or Person B are or are not owners of the directories containing the files in the CL? Does the author of the CL have to be one of the people doing a +1?

No, you usually won't Code-Review +1 your own CLs, that is sign that will be given by whoever is reviewing

your change.

you rarely should Code-Review your own CLs.  the only time it makes sense is in a TBR scenario.

-mike

[Will Harris]

Thanks for all the replies everyone. I think what's confusing is that there is no CR+2 on Chromium gerrit code review... and yet I'm hearing from multiple people that reviewers should be doing CR+2... (??)

I'm just going to go with what a few people said which I think can be summarized as:

"Chromium doesn't use +2 for Code-Review. For Chromium, +1 = lgtm and -1 = not lgtm and that's it. The CQ field is separate from Code-Review and there +1 = dry-run (git cl try) and +2 = submit to CQ (git cl set-commit)."

and 

"CQ +1 = dry run (git cl try)

CQ +2 = submit to CQ (git cl set-commit)

Code-Review -1 = NOT LGTM

Code-Review +1 = LGTM"

Thanks,

Will

[Aaron Gable]

And just reiterating for the sake of clarity:

On Tue, May 23, 2017 at 6:52 PM Mike Frysinger <vap...@chromium.org> wrote:

On Tue, May 23, 2017 at 9:33 PM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:14 PM, Will Harris <w...@chromium.org> wrote:

On Wed, May 24, 2017 at 11:10 AM, Thiago Farina <tfa...@chromium.org> wrote:

On Tue, May 23, 2017 at 10:04 PM, Will Harris <w...@chromium.org> wrote:

Hi, I read the FAQ at https://polygerrit.appspot.com/ and have started seeing a lot more Gerrit CLs pass by my way, but I feel I don't really fully understand the new code review system. In particular the difference between "Code Review" and "Commit Queue" and +1 vs +2 and -1 and the different buttons available in the PolyGerrit UI. Sorry in advance for the dumb questions, since I'm new to using Gerrit.

What is the difference between code review +1 and code review +2?

Does it take Code Review +1 or +2 to approve a CL?

Code-Review +1 means Looks good but someone else should approve.

Code-Review +2 means Looks good, approve.

If person A does a +1 and person B also does a +1 then is this a +2?

No, I don't think it sums up like this.

correct -- don't look at these as any sort of summing operation.  they could be called Code-Review-Foo and Code-Review-Bar and have the same relationship.

The way I see, +1 is usually from your team mate reviewing your patch, the +2 will come from the owner

of the code you are changing.

that's one approach when OWNERS are available.  if you mouse over each label, it gives you a description of the meaning.

CR-2 = Do not submit

CR-1 = I would prefer you didn't submit this

CR-0 = No opinion

CR+1 = Looks good to me, but someone else must approve

CR+2 = Looks good to me, approved

All of this talk of Code-Review-2 and Code-Review+2 does not apply to chromium/src.git. It applies to ChromeOS, Android, and some other projects that have been using Gerrit for a long time. At some point in the future, it may apply to Chromium (but you'll get lots of warning if it does).

For now, chromium folks only need to worry about CR-1 (equivalent to Rietveld's "not lgtm") and CR+1 (equivalent to Rietveld's "lgtm"). 

[Mostyn Bramley-Moore]

Could we add a link to https://chromium-review.googlesource.com from https://www.chromium.org/developers/contributing-code ?  And perhaps excerpts from this announcement?  

-Mostyn.

[Christian Biesinger]

So... If everything rebases the patch automatically, why is there a rebase button in the UI? When would it make sense to press it?

Thanks,

Christian

[Mike Frysinger]

it depends on the repo and the CQ bot and what you want to do.  gerrit allows you to select the merge strategy (cherry-pick, rebase, merge, etc...) when submitting.  and if you uploaded a series of changes but wanted to break the dep chain for one, you could rebase a CL on top of another point (like master).

-mike

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAPTJ0XFWkT0vfWJkNLLNNXBJyKU-zVs9S%2BfmrXA3e7SgZV1Zjw%40mail.gmail.com.

[Johnny Stenback]

I have no experience with Gerrit yet. However from reading this thread it makes it feel like the numeric labels leave a lot of room for mistakes that could be eliminated if we eliminated the numeric labels and replaced them with labels whose names were descriptions of the resulting actions/requests in this project. Would that be a reasonable change to request here?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAbOScn5bf2Embm0%3D1Pi%3D%3DnCtNevYF85L%3Do_4hzEitPT02D--w%40mail.gmail.com.

[Thiago Farina]

On Wed, May 24, 2017 at 11:17 AM, Johnny Stenback <jste...@chromium.org> wrote:

I have no experience with Gerrit yet. However from reading this thread it makes it feel like the numeric labels leave a lot of room for mistakes that could be eliminated if we eliminated the numeric labels and replaced them with labels whose names were descriptions of the resulting actions/requests in this project. Would that be a reasonable change to request here?

There is a tooltip when you hover the buttons that explains what they do. It is not all that confusion and complicated. One or two days of using it, and you will get used to it.

There are many teams already using the numeric system (and for many years), so changing to something else would be hard to push, I think.

--

Thiago Farina

[Mike Frysinger]

you'd be hiding fundamental aspects of gerrit which would simply be delaying the confusion to later on.

there is a `gerrit` tool in chromite which lets you set labels and their values.  using names won't work.

the gerrit search allows you to check label numeric values.  using names won't work.

-mike

[Christian Biesinger]

OK, thanks, so the summary is: The rebase button *for
chromium/src.git* is only useful in special situations mostly
involving dependent patchsets? Whereas, in other repos it is more
useful particularly if they default to a "merge" CQ strategy.

Christian

[Aaron Gable]

@mostynb: I have a bug on file against myself to do exactly that, it will definitely be done soon.

@cbiesinger: Because Chromium has a CQ which rebases for you during the submission process, you're right, the Rebase button is less likely to be useful. However, it can still be useful for (as Mike said) breaking chains of dependent CLs, since you can rebase a previously-dependent change on top of master instead. (You can also create chains of dependent CLs by rebasing on top of another change's ref, but that's usually not necessary).

@jstenback: As Thiago and Mike said (and as I said yesterday), the numeric labels are a fundamental aspect of how Gerrit works internally. Hiding the numbers without changing the internals would be an exercise in futility; changing the internals would require a lot of work and coordination with the thousands of open-source users who have their own Gerrit installations and configurations and workflows. We've already had extensive discussions with other Chromium engineers on this topic and decided that the current strategy is the best way to proceed. Yes, we're unfortunately asking you to learn something new during this transition, but we don't think it will be much extra mental load in the long run. Please give it a shot and see what you think during actual use :)

@vapier: Please be careful about giving ChromeOS-specific advice (e.g. Code-Review+2 and tools from chromite) on a thread specifically targeted at Chromium engineers migrating from Rietveld, for whom that information isn't relevant.

Aaron

[Mike Frysinger]

`gerrit` isn't CrOS specific by design.  anyone can run it against any GoB instance.  in fact, it works just fine with a chromium checkout:

$ ./third_party/chromite/bin/gerrit ...

wrt Code-Review+2, that isn't really CrOS specific.  if you want to say "Code-Review+2 isn't used (currently?) in chromium/src.git", then fine.  but it's a generic label that shows up outside of the chromiumos/ namespace.

-mike

[Anthony Berent]

Is there (or are there any plans for) an equivalent of Chromite Butler for Gerrit? Trying to land my first Chrome Gerrit CL I find it anoying that I can no longer see which files still need code-review +1 (LGTM).

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAbOSc%3DUP4K4B9ZMEcSQO%3DTeZmr0FSx1EpT_4yhpQ-M0%2BdR2%3Dw%40mail.gmail.com.

[Aaron Gable]

The Chromite Butler is a chrome extension which is neither owned nor maintained by the Chrome or ChromeOS infra teams. Please speak to sadrul@ if you want them to port the Chromite Butler to Gerrit.

On that note, yes, we do have plans to do some of the things that the Chromite Butler does (like suggesting OWNERS and highlighting files that haven't had owners review yet) with a Gerrit plugin, but doing so is not a prerequisite for switching away from Rietveld.

Aaron

[Nicholas Bishop]

> There is a tooltip when you hover the buttons that explains what they do. It is not all that confusion and complicated. One or two days of using it, and you will get used to it.

Perhaps the tooltips could be complimented with documentation links near the buttons? For example, the "+1" verified button's tooltip is just the word "Verified", which doesn't help with understanding what it is for or what it does.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2cYWVo0gQJAjmgTEeXFXUANZRyoXi%2B24W9QvGyxN4iMMg%40mail.gmail.com.

[Emil A Eklund]

On Thu, May 25, 2017 at 2:07 PM, Nicholas Bishop <nbi...@neverware.com> wrote:
>> There is a tooltip when you hover the buttons that explains what they do.
>> It is not all that confusion and complicated. One or two days of using it,
>> and you will get used to it.
>
> Perhaps the tooltips could be complimented with documentation links near the
> buttons? For example, the "+1" verified button's tooltip is just the word
> "Verified", which doesn't help with understanding what it is for or what it
> does.

-1/0/+1 makes sense in a way but the distinction between +1 and +2,
especially for the CQ, is super confusing. Given that we already have
the descriptive text why can't we add that to the buttons? I
understand that you might not want to hide the number entirely (for
internal gerrit reasons) but something like "Needs work (-1)",
"Neutral (0)", "LGTM (+1)" and "Approved (+2)" for Review and "Commit
(+1)" and "Dry run (+2)" for the CQ would be very helpful.

[Christian Biesinger]

One more question -- I noticed Gerrit has an "assigned" field. Is that used for anything for chromium/src.git or should I just ignore it?

In general, what is that intended for in a way that is different from patch author and reviewers?

Thanks!

Christian

[Mike Frysinger]

the old UI used to show that, but i think the PG UI killed it off as a simplification


-mike

[Aaron Gable]

Emil and Mike: I've filed a bug to have those textual labels show up to the right of the buttons in the reply dialog. The textual labels already show up as tooltips and are read by screenreaders.

Christian: You don't need to worry about the assigned field, we don't have any policies in place saying that people should use it (since Rietveld didn't have an equivalent). It's used to say "this is the person who should take the next action on this change". You're certainly welcome to use it, if you feel like attracting the attention of a specific one of your reviewers.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAAbOSckr%3DiwXprwpP59rEKs%3DsuCj9JSGPTjm-tvByAReGMwtKA%40mail.gmail.com.

[Aleksandr Derbenev]

Hi.

Does gerrit settings set for open-source contributors set fine?

I made a CL to chromium/src and received CR+1 on it (https://chromium-review.googlesource.com/c/517163/). But I have no permissions to set CQ-label.

UI says: "You don't have permission to edit this label."

git cl try/git cl set-commit says: "Forbidden: applying label "Commit-Queue": 1 is restricted."

-- 

Aleksandr Derbenev

Yandex LLC.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---

You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

[Aaron Gable]

Yes, in order to run the commit queue, you need to have trybot access (i.e. permission to run code on our infrastructure). This access can be granted easily. In Rietveld, you could have clicked the Commit Queue checkbox, but the CQ would have immediately rejected your run. In Gerrit, we simply prevent people without trybot access from setting CQ+1 at all, to prevent that unnecessary round trip.

[Aleksandr Derbenev]

Yes, in order to run the commit queue, you need to have trybot access (i.e. permission to run code on our infrastructure). This access can be granted easily. In Rietveld, you could have clicked the Commit Queue checkbox, but the CQ would have immediately rejected your run. In Gerrit, we simply prevent people without trybot access from setting CQ+1 at all, to prevent that unnecessary round trip.

Rietvield does not reject my runs, but gerrit does. Trybot access grants in gerrit are not same, are they?

-- 

Aleksandr Derbenev

http://staff/alex-ac/

[Lucas Gadani]

In this case, the CL already has LGTM from an owner, I think we should allow the external contributors to submit to CQ even without having tryjob access.

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2f96xdaFjECfC9VqxC0zd5X2%3DedQh7xKkFQv5jaO_GqkA%40mail.gmail.com.

[Aaron Gable]

Sorry, you're completely right. I had the permissions for the Commit-Queue label misconfigured from what this design doc (sorry, internal only) laid out. I've fixed the issue, and anyone should be able to set the Commit-Queue label now. The CQ process itself will determine whether or not to run tests based on who the uploader is, who the run requester is, and what reviewers have (or have not) approved the change.

[Daniel Murphy]

Is there a way to collapse or split up the 'automated' messages that aren't published comments? It's rather noisy. Feature request?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fhuA4ZLphHh9hHG2kFby_AAn9NpqvY7XCW8ZsjycWdaw%40mail.gmail.com.

[Primiano Tucci]

On Wed, May 31, 2017 at 1:25 AM Daniel Murphy <dmu...@chromium.org> wrote:

Is there a way to collapse or split up the 'automated' messages that aren't published comments? It's rather noisy. Feature request?

 

+1, the SNR of gerrit CLs is way too low and I feel I lose track of real comments. Example from today (https://chromium-review.googlesource.com/c/517690/):

The only comment from a human in that list of 9 messages is "How does this look" in the middle. All the rest is process-related noise. If some other reviewer did add some comment I would have probably accidentally missed it.

Filed https://bugs.chromium.org/p/gerrit/issues/detail?id=6354

P.S. Also the same "Commit bot" has two different avatars, and in one case (CQ +1) it just gets the same anonymous avatar of the author.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32m%2BSg-1tMXk_VJQJgdkMcA19e5%2BsrFq1hi7Lw4V9DfYA%40mail.gmail.com.

[Christian Biesinger]

There is a "Show comments only" link on the right that helps somewhat, though there's still some messages that probably should be hidden.

Christian

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2f96xdaFjECfC9VqxC0zd5X2%3DedQh7xKkFQv5jaO_GqkA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fhuA4ZLphHh9hHG2kFby_AAn9NpqvY7XCW8ZsjycWdaw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32m%2BSg-1tMXk_VJQJgdkMcA19e5%2BsrFq1hi7Lw4V9DfYA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2ByH71cx3h64M0g6BZ_BHvjPFz0zFfoBadTcSQ62W92VLTc1eg%40mail.gmail.com.

[Primiano Tucci]

On Wed, May 31, 2017 at 3:28 PM Christian Biesinger <cbies...@chromium.org> wrote:

There is a "Show comments only" link on the right that helps somewhat, though there's still some messages that probably should be hidden.

That:

1) Doesn't seem to work properly as half of the autogenerated messages stay there, as you point out

2) Should be the default, to match today's rietveld behavior

(Bugs aside, which are kinda unavoidable in any new system, thanks a lot to everybody involved for the hard work here and for being responsive on the list)

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2f96xdaFjECfC9VqxC0zd5X2%3DedQh7xKkFQv5jaO_GqkA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fhuA4ZLphHh9hHG2kFby_AAn9NpqvY7XCW8ZsjycWdaw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32m%2BSg-1tMXk_VJQJgdkMcA19e5%2BsrFq1hi7Lw4V9DfYA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2ByH71cx3h64M0g6BZ_BHvjPFz0zFfoBadTcSQ62W92VLTc1eg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAPTJ0XGkpGi9_37cqipD3oPctufkbnHS4BZDxM_OY2bate11ng%40mail.gmail.com.

[Sylvain Defresne]

Why is patchset title restricted to alphanumeric chars and space? It prevents me from using proper comments for my patchset (for example "address eugenebut's comments").

-- Sylvain

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAPTJ0XGkpGi9_37cqipD3oPctufkbnHS4BZDxM_OY2bate11ng%40mail.gmail.com.

[Aaron Gable]

dmurphy, primiano, and cbiesinger: Yep, we know the messages at the bottom are less helpful than we'd like them to be. We're doing a few things to help with that:

* Hiding autogenerated messages (like "uploaded new patchset") by default

* Having smarter support for deciding which messages should be hidden/shown/expanded

* Merging "added reviewer" and "left a comment" messages into a single message, when those happen at the same time

sdefresne: Yeah, that one really sucks. It's because of the protocol that is used to set the patchset title: it is appended to the magic refs/changes/* refs that Gerrit uses to receive CLs, but Git has limitations on the characters that can be in ref names. We are working on updating to use the newer protocol (git push options), but doing so requires updating the version of git we ship in depot_tools to be >2.10, so we're working on that first. We could instead use the REST API to set the patchset title after uploading, but it would add latency to git-cl-upload; would that be preferable?

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--

You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2f96xdaFjECfC9VqxC0zd5X2%3DedQh7xKkFQv5jaO_GqkA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fhuA4ZLphHh9hHG2kFby_AAn9NpqvY7XCW8ZsjycWdaw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32m%2BSg-1tMXk_VJQJgdkMcA19e5%2BsrFq1hi7Lw4V9DfYA%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2ByH71cx3h64M0g6BZ_BHvjPFz0zFfoBadTcSQ62W92VLTc1eg%40mail.gmail.com.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

[Colin Blundell]

Since we're noting problems (BTW, I've also hit the alphanumeric issue), I've noticed a consistent problem with the intersection of Gerrit and my workflow over the past few days. Maybe I'm just holding it wrong. Here's my workflow for responding to reviews, and how it intersects with Gerrit:

(1) I have a CL open with review comments

(2) I have a terminal open with the code

(3) I respond to comments as I make changes to the code.

(4) At some point I upload the new patchset and potentially continue to respond to comments in the window from (1)

(5) A small floatover appears in the bottom left of the window saying "There is a new uploaded patchset" with a button to press that says "Reload". I typically don't notice this until hitting (6) below.

(6) Once (5) appears, all my work on responding to comments seems to be impossible to save: The only thing I can do is hit "Reload", at which point all my comment drafts go away and have to be redone.

Am I missing something?

Thanks,

Colin

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fk17%3DmMuWJU8E1z3ZNH7avHftzB%3DG_GED1OfzzFo5wgw%40mail.gmail.com.

[Aaron Gable]

Great question! Like Rietveld, you can leave comments on any patchset. When you reload the page (either via the popup toast, or via normal means), it displays the latest patchset by default. But all of your comments on older patchset -- whether they're published already, unpublished replies to previous comments, or unpublished new line comments -- are still accessible on that old patchset. You can view them using the dropdown in the upper left corner of the file list; it even tells you which patchsets have comments on them already. And when you open the Reply dialog, the draft comments you've already written should be there, ready to be published.

If I've misunderstood, or something deeper is going wrong, please file a bug and we'll continue discussion there!

Aaron

[Stefan Zager]

Apologies if this has already been discussed, but...

I'm on the watchlist for some code reviews, but I have a gmail filter to label code reviews on which I am a reviewer, to distinguish them from the ones where I'm just on the watchlist:

Has the words: "Reviewers: .* szager"

I'm having trouble creating the right filter for gerrit.  I can do this:

Has the words: would like .*Stefan Zager.* to review this change.

... but that only catches reviews where I am in the initial list of reviewers.  If I am subsequently added as a reviewer, then this filter doesn't work.

I also tried:

From: gerrit-noreply

To: sza...@chromium.org

... but this unfortunately also matches my watch list CL's (which have cc: szager+layoutwatch@chromium.org).

Any guidance on how to write a gmail filter for this?

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

--

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2fsORHinkZ%3Dx2SK-8DehMgSpiw7PK8oe3%3DKUEFvPzdZ6A%40mail.gmail.com.

[Aaron Gable]

Yep, we lay out some tips for gmail/inbox filters in the faq. Basically the idea is to use the gerrit footers (which you can see if you click "View Original"), rather than the naively-visible content of the email itself.

To get the changes which have you as a review, but which you didn't upload, you want to filter on includes 'Gerrit-Reviewer: Your Name' and excludes 'Gerrit-Owner: Your Name'

On Wed, May 31, 2017 at 3:01 PM Stefan Zager <sza...@chromium.org> wrote:

Apologies if this has already been discussed, but...

I'm on the watchlist for some code reviews, but I have a gmail filter to label code reviews on which I am a reviewer, to distinguish them from the ones where I'm just on the watchlist:

Has the words: "Reviewers: .* szager"

I'm having trouble creating the right filter for gerrit.  I can do this:

Has the words: would like .*Stefan Zager.* to review this change.

... but that only catches reviews where I am in the initial list of reviewers.  If I am subsequently added as a reviewer, then this filter doesn't work.

I also tried:

From: gerrit-noreply

To: sza...@chromium.org

... but this unfortunately also matches my watch list CL's (which have cc: szager+la...@chromium.org).

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

--

[Olga Sharonova]

I find the font used for the source code quite difficult to read due to its anti-aliasing. No matter what font size I use - It just looks blurry to me.

For me it's quite an issue and I filed a bug - welcome to star it if you experience the same problem as well.

Thanks,

Olga

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

--

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAH58R2cCqkREJRDs_dLjfurSHf_8rKcq1998z3uCcKhmLK9iaA%40mail.gmail.com.

[Aaron Gable]

Thanks for filing that bug! I added some screenshots and it'll definitely be taken into consideration.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

--

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHri2_Tw26ZPdZyfFO_rw%2BT0ek0hqqsDs%3Dnwq_jhLFNARoJ3Rw%40mail.gmail.com.

[Colin Blundell]

Yes, I confirmed that this works for me in the workflow I described below. Thanks, Aaron!

[PhistucK]

How will crrev.com review links be managed? The difference between a commit position and a review number was commit position being up to six or seven digit long and the review number being a very long number. Gerrit review numbers are much smaller -

https://chromium-review.googlesource.com/c/518552/

Will crrev.com accommodate them in a different way (crrev.com/r/review-number)?

It always was a bit sketchy to use the same field for commit positions and review numbers, so that makes sense to me.

☆PhistucK

On Fri, Jun 2, 2017 at 9:47 PM, 'Daniel Murphy' via blink-dev <blin...@chromium.org> wrote:

Thanks for all your work here Aaron, I'm really happy with the results and I'm excited for the future!

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

--

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32b848bRgjktVO4HM%3D5%3DmfbvLcPiLviKUUUTzXXWrFY8Q%40mail.gmail.com.

[Michael Giuffrida]

On Mon, Jun 5, 2017 at 9:33 PM PhistucK <phis...@gmail.com> wrote:

How will crrev.com review links be managed? The difference between a commit position and a review number was commit position being up to six or seven digit long and the review number being a very long number. Gerrit review numbers are much smaller -

https://chromium-review.googlesource.com/c/518552/

Will crrev.com accommodate them in a different way (crrev.com/r/review-number)?

It always was a bit sketchy to use the same field for commit positions and review numbers, so that makes sense to me.

close: crrev.com/c/review-number

or crrev.com/i/internal-review-number

https://polygerrit.appspot.com/ has some justification for not using top-level crrev.com/ for this.

 

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

--

To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CA%2B4qT32b848bRgjktVO4HM%3D5%3DmfbvLcPiLviKUUUTzXXWrFY8Q%40mail.gmail.com.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABc02_LKLgeKgKZ-CQ3URNnDv6Qtdkw7PisF7E-3hd1tqH0YjQ%40mail.gmail.com.

[Stefan Zager]

Apologies (again) if this has been discussed already, but:

Is there any way to tell which version of a patch was used for a try job?  In Rietveld, try job results were displayed along with a particular patch set, but now there's just one box for try jobs results.

You received this message because you are subscribed to the Google Groups "infra-announce" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-announce+unsubscribe@chromium.org.
To post to this group, send email to infra-a...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-announce/CAH58R2fD8yys%2BJZVL9ZZnCvMU9QcAq5mLAfsMQpAcyqLTYd8AQ%40mail.gmail.com.

--
--
Chromium OS Developers mailing list: chromiu...@chromium.org

View archives, change email options, or unsubscribe:

http://groups.google.com/a/chromium.org/group/chromium-os-dev?hl=en

[Aaron Gable]

The tryjobs that are displayed actually are displayed for the specific patchset that you're viewing. If you upload a change and it fails a bunch of bots, but then upload a fix in the second patchset and retry those bots, the old failures will disappear. They'll re-appear if you use the patchset selector dropdown to view the older patchset.

The one difference is that Gerrit recognizes that tryjobs for one patchset are probably relevant for subsequent patchsets that don't change anything meaningful -- commit description changes and trivial rebases (i.e. no conflicts to resolve). So sometimes it will display trybot results from an older patchset, but only if that older patchset is functionally identical to the one you're looking at now.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-announc...@chromium.org.

To post to this group, send email to infra-a...@chromium.org.

[Stefan Zager]

On Wed, Jun 7, 2017 at 11:58 AM, Aaron Gable <aga...@chromium.org> wrote:

The one difference is that Gerrit recognizes that tryjobs for one patchset are probably relevant for subsequent patchsets that don't change anything meaningful -- commit description changes and trivial rebases (i.e. no conflicts to resolve). So sometimes it will display trybot results from an older patchset, but only if that older patchset is functionally identical to the one you're looking at now.

Hmm... not my experience.  I uploaded a new patch set that fixed a syntax error, and the old try jobs results (all red due to compile failures) showed up.

Any known issues along these lines?

[Aaron Gable]

Nope, no known issues like that at the moment. (There used to be a known issue about showing stale data due to credentials not refreshing, and there's currently a known issue about showing the wrong thing on submitted changes in non-chromium repos, but nothing for what you're describing.) Please file a bug with a link to the reproduction case!

[Jianpeng Chao]

Anyone know what is the checkbox beside the filename used for?

[Daniel Cheng]

I use it for tracking which files in a CL I've reviewed. I believe it's also auto checked if you open the diff for that file in the file by file view (not just expand the diffs inline on the main review page).

Daniel

On Mon, Jun 26, 2017, 20:52 'Jianpeng Chao' via blink-dev <blin...@chromium.org> wrote:

Anyone know what is the checkbox beside the filename used for?

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/45f52360-c755-4968-825a-872864ee51f4%40chromium.org.

[Kyle Charbonneau]

Would it be possible to block submit if there are unresolved comments? It seems like it would provide a couple benefits without major downsides.

• It's easy to miss comments on old patch sets. It would ensure no comments get overlooked.
• It provides a better way to "+1 Code Review with nits". The author has to explicitly resolve the nit comments before they can submit.
• This would be consistent with other Google code review tools.
• Gerrit has support for resolved/unresolved comments and automatically marks comments as resolved for "Done" and "Ack" quick replies.
  
• Gerrit seems to have a simple way to block submit on unresolved comments. https://gerrit-review.googlesource.com/Documentation/prolog-cookbook.html#_example_16_make_change_submittable_if_all_comments_have_been_resolved

The only real downside is users have to get used to a slight change in workflow and mark comments as resolved when replying. Has this been considered?

[Aaron Gable]

(whoops, from the right account this time, sorry if you still both)

Yep, it's a thing we've considered! We're trying to make as few workflow changes as possible during this transition, so we're not going to do so right now. But we're aware of the possibility and will be considering it as a future improvement once everyone has moved to Gerrit and things are stable.

Please file a bug with the labels Proj-Gerrit-Migration and Milestone-Afterglow so that others can chime in on whether they think this would be a good improvement to make in the future, and to help us prioritize it relative to other post-launch work.

Thanks!

On Wed, Jun 28, 2017, 08:40 Aaron Gable <aga...@google.com> wrote:

Yep, it's a thing we've considered! We're trying to make as few workflow changes as possible during this transition, so we're not going to do so right now. But we're aware of the possibility and will be considering it as a future improvement once everyone has moved to Gerrit and things are stable.

Please file a bug with the labels Proj-Gerrit-Migration and Milestone-Afterglow so that others can chime in on whether they think this would be a good improvement to make in the future, and to help us prioritize it relative to other post-launch work.

Thanks!

[Andrew Grieve]

Along the same lines, in Rietveld there's handy coloured boxes for each file that show if you've selected appropriate OWNERS for each (I think this is actually added by an extension), along checkboxes for which of the suggested OWNERs to include. 

Is it in the roadmap to have that as well? I find it comparatively tedious to choose owners with Gerrit.

--

--
Chromium Developers mailing list: chromi...@chromium.org

View archives, change email options, or unsubscribe:

http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2ft9fxguVLMEGVF3F53Rxkn0v6v4G0gp35AwenF-PxuBw%40mail.gmail.com.

[Jacob Dufault]

Is it possible to delete an older patch inside of a CL? I often upload a change, notice a small typo, and reupload the patch. In retvield I would delete the now unseen change as it does not add any value for the reviewer. I haven't been able to find similar functionality in gerrit. It seems like the 'drafts' feature might work, but that is deprecated.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABiQX1Uhz5P9j%3DmV7QTeWOcqGiTc8O_o4MXomCoMz26irJ0k1Q%40mail.gmail.com.

[Kyle Charbonneau]

Please file a bug with the labels Proj-Gerrit-Migration and Milestone-Afterglow so that others can chime in on whether they think this would be a good improvement to make in the future, and to help us prioritize it relative to other post-launch work.

Filed crbug.com/737646.

Kyle

On Wed, Jun 28, 2017 at 11:44 AM, Aaron Gable <aga...@chromium.org> wrote:

[Michael Giuffrida]

In Rietveld, I've lost drafts when people have done this. It's less likely if you're immediately re-uploading a patch, but still possible. IDK if it works the same in Gerrit, or if drafts are retained in deleted patch sets, but there's not much harm in keeping an older patch around.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFxOgiVEnYuvPQ6sj%3DLxE43MoiyTipPp%2BbZEqXoxY0uvpbry9A%40mail.gmail.com.

[Jacob Dufault]

> In Rietveld, I've lost drafts when people have done this. It's less likely if you're immediately re-uploading a patch, but still possible. IDK if it works the same in Gerrit, or if drafts are retained in deleted patch sets, but there's not much harm in keeping an older patch around.

I find it trickier to compare between two patches (ie, what changed since I last reviewed) when there are lots of intermediate patches. This could probably be easily fixed in UI by displaying an icon in the dropdown signifying that the patch has comments.

[Daniel Cheng]

On Wed, Jun 28, 2017 at 11:26 AM 'Jacob Dufault' via Chromium-dev <chromi...@chromium.org> wrote:

> In Rietveld, I've lost drafts when people have done this. It's less likely if you're immediately re-uploading a patch, but still possible. IDK if it works the same in Gerrit, or if drafts are retained in deleted patch sets, but there's not much harm in keeping an older patch around.

I find it trickier to compare between two patches (ie, what changed since I last reviewed) when there are lots of intermediate patches. This could probably be easily fixed in UI by displaying an icon in the dropdown signifying that the patch has comments.

Incidentally, there's a bug for that: https://bugs.chromium.org/p/gerrit/issues/detail?id=6486

Daniel

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFxOgiUUwn96YtbNp683B5C%2B%2Boh%2BaGwC7m90bupAtVb62-Kg%3DQ%40mail.gmail.com.

[Colin Blundell]

I have also missed the functionality of being able to delete patch sets. I use this only to clean up my patch sets before sending out an updated CL for re-review, which isn't vulnerable to the problem Michael's describing.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACi5S_3Yn8G9U1zXZWEnVzVRTEL9v-8dtnU9pMBjAESkw-FAaA%40mail.gmail.com.

[dan...@chromium.org]

On Wed, Jun 28, 2017 at 3:20 PM, Colin Blundell <blun...@chromium.org> wrote:

I have also missed the functionality of being able to delete patch sets. I use this only to clean up my patch sets before sending out an updated CL for re-review, which isn't vulnerable to the problem Michael's describing.

Same, though I would settle for renaming them. I haven't seen a way to do that either.

 

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABiQX1Uhz5P9j%3DmV7QTeWOcqGiTc8O_o4MXomCoMz26irJ0k1Q%40mail.gmail.com.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAFxOgiVEnYuvPQ6sj%3DLxE43MoiyTipPp%2BbZEqXoxY0uvpbry9A%40mail.gmail.com.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CACi5S_3Yn8G9U1zXZWEnVzVRTEL9v-8dtnU9pMBjAESkw-FAaA%40mail.gmail.com.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGE5NGJ2Pdb4zoGV71g9vBPy6Uhr%2B2EiRmpyspEohMeKVpr2A%40mail.gmail.com.

[Dan Sinclair]

You can click on the patchset name in the Patch set <combobox> / hash
/ Download / <name> section and it becomes editable, hitting enter
seems to save it (although I don't see any way to cancel without
reloading the page?

dan

>>>>> an email to chromium-dev...@chromium.org.

> --
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
> ---
> You received this message because you are subscribed to the Google Groups
> "Chromium-dev" group.
> To view this discussion on the web visit

> https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAHtyhaSjmNURtiFKZ26gtaNeCi1Ge86GnnL8MahB1i0%3DH6qeYQ%40mail.gmail.com.