PSA: Recording which Rietveld patchset is commited and reverted

[rmi...@chromium.org]

The CQ now records in Rietveld which patchset was committed. This is what the new comment by chrome-bot looks like:

"Committed patchset #6 (100001) as 12345"

Note: 100001 here stands for the patchset id in Rietveld's datastore, this will be useful for auditing in cases where patchsets are deleted and recreated. We plan to remove this from the commit message once I add logging to record when patchsets are deleted.

Using the one-click revert button on Rietveld will now also output which patchset was reverted, the patchset number will be displayed in the inverted CL's description as well.

Example CLs:

* https://codereview.chromium.org/156683002/ (original CL)

* https://codereview.chromium.org/461173002/ (revert CL)

These new messages should be particularly useful for CLs that are committed, reverted, worked on again, and recommitted.

This work was tracked in chromium:308792 and chromium:402534.

Thanks!

[Dirk Pranke]

Hm. I've always kinda felt like it's bad style to commit the same CL more than once, and encouraged people to open new CLs. If you re-use the same CL, I find it hard to follow the history of things. (Where necessary, you can address the problem of carrying over the lgtms from the prior CL by just using TBR).

Am I in the minority here?

-- Dirk

[Nico Weber]

I think this has been discussed a few times, and there were people in both camps.

[Viet-Trung Luu]

On Thu, Aug 14, 2014 at 10:06 AM, Dirk Pranke <dpr...@chromium.org> wrote:

The great advantage of reusing the same CL (esp. to reviewers) is that it makes it much easier to see what's being changed (vs the original patch, which was already lgtmed) in the re-land.

Am I in the minority here?

-- Dirk

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

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

[Stuart Morgan]

On Thu, Aug 14, 2014 at 10:55 AM, Viet-Trung Luu <viettr...@chromium.org> wrote:

The great advantage of reusing the same CL (esp. to reviewers) is that it makes it much easier to see what's being changed (vs the original patch, which was already lgtmed) in the re-land.

The best practice for the alternate approach is that you start the fresh CL by uploading the original patch, then you upload the fixes, and in the review message you say something like "Patch set 1 is the original patch, patch set 2 has the fixes". This gives the same advantage, and it's simple to do since the first step of fixing it is to re-apply the same patch locally anyway (you just have to upload as part of that setup).

-Stuart 

[Scott Hess]

+1 to this, I've had people do it to deal with reverts and it's very nice.  In some cases it can even improve things over using the original CL if there have since been other changes in the files.

-scott