--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
I find this one a bit noisy. I often use Rietveld as a visual diff tool because I can't read the output of `git diff` very effectively.So when I upload to self-review, my reviewer gets spammed right away (before I publish that I've addressed nits, etc.) presumably opening up the CL to look when it's not necessarily ready yet.If the motivation is safety (i.e. to avoid changes being snuck in), would it be possible to instead notify when the CL lands that what landed was different, rather than when a new patchset is uploaded?
On Thu, Feb 5, 2015 at 10:25 AM, Scott Graham <sco...@chromium.org> wrote:I find this one a bit noisy. I often use Rietveld as a visual diff tool because I can't read the output of `git diff` very effectively.So when I upload to self-review, my reviewer gets spammed right away (before I publish that I've addressed nits, etc.) presumably opening up the CL to look when it's not necessarily ready yet.If the motivation is safety (i.e. to avoid changes being snuck in), would it be possible to instead notify when the CL lands that what landed was different, rather than when a new patchset is uploaded?+1, I agree with all this.
On Thursday, February 5, 2015 at 1:27:56 PM UTC-5, Nico Weber wrote:On Thu, Feb 5, 2015 at 10:25 AM, Scott Graham <sco...@chromium.org> wrote:I find this one a bit noisy. I often use Rietveld as a visual diff tool because I can't read the output of `git diff` very effectively.So when I upload to self-review, my reviewer gets spammed right away (before I publish that I've addressed nits, etc.) presumably opening up the CL to look when it's not necessarily ready yet.If the motivation is safety (i.e. to avoid changes being snuck in), would it be possible to instead notify when the CL lands that what landed was different, rather than when a new patchset is uploaded?+1, I agree with all this.I expected lgtm w/nits to be resolved in the next uploaded patchset but, as you mentioned, this is not always the cause. I am though not sure how to work around this.
Or when the issue is closed?On Thu, Feb 5, 2015 at 12:01 PM, Nico Weber <tha...@chromium.org> wrote:On Thu, Feb 5, 2015 at 11:59 AM, Ravi <rmi...@chromium.org> wrote:
On Thursday, February 5, 2015 at 1:27:56 PM UTC-5, Nico Weber wrote:On Thu, Feb 5, 2015 at 10:25 AM, Scott Graham <sco...@chromium.org> wrote:I find this one a bit noisy. I often use Rietveld as a visual diff tool because I can't read the output of `git diff` very effectively.So when I upload to self-review, my reviewer gets spammed right away (before I publish that I've addressed nits, etc.) presumably opening up the CL to look when it's not necessarily ready yet.If the motivation is safety (i.e. to avoid changes being snuck in), would it be possible to instead notify when the CL lands that what landed was different, rather than when a new patchset is uploaded?+1, I agree with all this.I expected lgtm w/nits to be resolved in the next uploaded patchset but, as you mentioned, this is not always the cause. I am though not sure how to work around this.What about notifying if a new patch set has been uploaded when cq is pressed?
On Thursday, February 5, 2015 at 3:02:50 PM UTC-5, Scott Graham wrote:Or when the issue is closed?On Thu, Feb 5, 2015 at 12:01 PM, Nico Weber <tha...@chromium.org> wrote:On Thu, Feb 5, 2015 at 11:59 AM, Ravi <rmi...@chromium.org> wrote:
On Thursday, February 5, 2015 at 1:27:56 PM UTC-5, Nico Weber wrote:On Thu, Feb 5, 2015 at 10:25 AM, Scott Graham <sco...@chromium.org> wrote:I find this one a bit noisy. I often use Rietveld as a visual diff tool because I can't read the output of `git diff` very effectively.So when I upload to self-review, my reviewer gets spammed right away (before I publish that I've addressed nits, etc.) presumably opening up the CL to look when it's not necessarily ready yet.If the motivation is safety (i.e. to avoid changes being snuck in), would it be possible to instead notify when the CL lands that what landed was different, rather than when a new patchset is uploaded?+1, I agree with all this.I expected lgtm w/nits to be resolved in the next uploaded patchset but, as you mentioned, this is not always the cause. I am though not sure how to work around this.What about notifying if a new patch set has been uploaded when cq is pressed?Checking for new patches when the CQ checkbox is clicked sgtm, since the intent is to now commit the patch. This will not however cover the 'git cl land' case unless doing that uploads the patch to Rietveld to check if it changed or not (jochen@ has spoken about this recently).I prefer this more than checking when the issue is closed because I know I have closed many issues that have not been committed and abandoned for various reasons.How do others feel about performing this check when CQ checkbox is clicked?
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
Can the "added after l-g-t-m" include the patchset title? I'm much more concerned about patches titled "fix crash" or "fix test failure" than "rebase" or "fix gn".
James
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
On Friday, February 6, 2015 at 12:14:22 PM UTC-5, James Cook wrote:Can the "added after l-g-t-m" include the patchset title? I'm much more concerned about patches titled "fix crash" or "fix test failure" than "rebase" or "fix gn".Including the patchset title (if it exists) in the message does sound useful. I will add it, thank you for the suggestion.
Nice to have this feature! Would it also be worth also sending
notifications when somebody deletes the latest patch set? Unless
reviewers look at all previous patch sets, it seems like it might
still be possible for an evil person to sneak a bad change in the
middle of a long series of patch sets, then delete the last couple of
patch sets post-LGTM.
--
--
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.
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 chromium-dev...@chromium.org.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.