"This CL has an open dependency", no it doesn't...!

[Gabriel Charette]

Can we please get rid of whatever prevents CLs with "dependencies" from going to CQ?

I really like that rietveld now tracks CLs with dependencies and allows tries which incorporate the dependent changes.

But sometimes I just chain branches locally because that's easier for whatever reason and preventing CQ on a CL that doesn't actually depend on its parent branch landing first is really a pain...

If we don't want to allow CQ'ing a CL with a parent change, can we at least allow manually stripping the parent post-upload.

Thanks,

Gab

[Thiago Farina]

CCing infra, since rietveld and cq seems more infra centered.

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

--

Thiago Farina

[Sylvain Defresne]

I second that. Currently, my workaround is to create an interstitial branch with a change to build/whitespace_file.txt that I do not upload. It's a bit of a pain and I'd like to be able to either pass --no-dependent-cl or remove it post-upload.

-- Sylvain

--

[Scott Graham]

+1.

I've been working around it by closing the "parent" issue, clicking CQ on the "child", waiting a while until it actually starts, and then reopening the "parent".

But for me the disallowing doesn't seem to accomplish anything useful, so it seems like unnecessary hassle.

--

[Dirk Pranke]

I don't think we should remove the feature altogether; it can prevent things from being sent to the CQ that really do depend on the prior CLs, and I feel like it's probably the right default.

I am fine w/ some sort of checkbox or something to remove the dependency, though. There could probably also be some way to tell git-cl not to mark the patch as actually dependent on the upstream branch (even though git thinks it is tracking an upstream branch).

-- Dirk

[Scott Graham]

On Thu, Apr 7, 2016 at 12:27 PM, Dirk Pranke <dpr...@chromium.org> wrote:

I don't think we should remove the feature altogether; it can prevent things from being sent to the CQ that really do depend on the prior CLs, and I feel like it's probably the right default.

The problem I have with it is that there's lots of other reasons that CLs can be co-dependent or not ready to land (for example the deletions you and I have open from yesterday!) and interdependent branches locally are a small number of those.

But if it really helps people, then yeah I guess a "be-quiet-I-know-what-I'm-doing" checkbox would be nice.

[Dirk Pranke]

On Thu, Apr 7, 2016 at 12:35 PM, Scott Graham <sco...@chromium.org> wrote:

On Thu, Apr 7, 2016 at 12:27 PM, Dirk Pranke <dpr...@chromium.org> wrote:

I don't think we should remove the feature altogether; it can prevent things from being sent to the CQ that really do depend on the prior CLs, and I feel like it's probably the right default.

The problem I have with it is that there's lots of other reasons that CLs can be co-dependent or not ready to land (for example the deletions you and I have open from yesterday!) and interdependent branches locally are a small number of those.

I'm not sure about the "small number" part; I would guess that those are the majority. Regardless, I think that even though we can't correctly track all of the dependencies, if we can track some of them that's better than nothing (as long as there is a way to opt out. There kind of is now, but we can obviously make it easier).

-- Dirk

[Aaron Gable]

I'm curious under what circumstances it is easier to chain the branches locally but have them not have dependencies on each other. Is it simply that "git checkout -tb foo" is ingrained in your muscle memory?

[Avi Drissman]

I do that all the time in refactoring, when removing a function. I make a branch and compile; 50 errors. I fix one directory with 15 of them, and make a CL. I then make a dependent branch, compile, get 35 errors, and fix 15 more in a new CL. The CLs are independent and can land separately, but it's useful to chain them locally.

Avi

[Dirk Pranke]

Often you might be working on some big complicated change or bug fix and get to a point where you break it up into smaller independent pieces for review. The easiest way to do that it to stack branches, but just because they're stacked doesn't mean they actually depend on their parents.

Or, sometimes you might create a new branch off of your existing branch for a minor change because doing so is easier than creating a new branch off of origin/master which might force a large recompile.

-- Dirk

[Aaron Gable]

Thanks! Yep, definitely seems like we should have manual "add/remove dependency" buttons in Rietveld. +Andrew Bonventre (Bons) 

[Yuta Kitamura]

"git branch --set-upstream-to=master" will make your branch directly depend on master, and I think git cl will respect it. Doesn't this work for you?

--

[Paweł Hajdan, Jr.]

+rmistry

Ravi, didn't you add this feature as part of https://bugs.chromium.org/p/chromium/issues/detail?id=504362 ?

I think we could use some help providing support for it.

Paweł

[Paweł Hajdan, Jr.]

+infra-dev again

[Ravi]

On Friday, April 8, 2016 at 5:29:39 AM UTC-4, Paweł Hajdan, Jr. wrote:

+infra-dev again

On Fri, Apr 8, 2016 at 11:28 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

+rmistry

Ravi, didn't you add this feature as part of https://bugs.chromium.org/p/chromium/issues/detail?id=504362 ?

Yes, the feature was launched around 10 months ago. Right after launch there were similar complains which is why I had added two ways to skip the dependency checks (they are detailed in the design doc here).

One way is to skip checks for specific branches with "git config branch.test.skip-deps-uploads True" and the other way (possibility more relevant to this thread) is to use "NO_DEPENDENCY_CHECKS=true" in the CL description.

I personally do not think we should add more UI elements to Rietveld for this similar to how we do not have them for the other CQ keywords.

[Colin Blundell]

On Fri, Apr 8, 2016 at 12:37 PM Ravi <rmi...@chromium.org> wrote:

On Friday, April 8, 2016 at 5:29:39 AM UTC-4, Paweł Hajdan, Jr. wrote:

+infra-dev again

On Fri, Apr 8, 2016 at 11:28 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

+rmistry

Ravi, didn't you add this feature as part of https://bugs.chromium.org/p/chromium/issues/detail?id=504362 ?

Yes, the feature was launched around 10 months ago. Right after launch there were similar complains which is why I had added two ways to skip the dependency checks (they are detailed in the design doc here).

One way is to skip checks for specific branches with "git config branch.test.skip-deps-uploads True" and the other way (possibility more relevant to this thread) is to use "NO_DEPENDENCY_CHECKS=true" in the CL description.

I personally do not think we should add more UI elements to Rietveld for this similar to how we do not have them for the other CQ keywords.

From upthread, I guess that these options are not listed in the error message. Why not just add something like:

"If you are sure that there is no real dependency, you can do XXX to land the CL."

[Ravi]

On Friday, April 8, 2016 at 7:34:16 AM UTC-4, Colin Blundell wrote:

On Fri, Apr 8, 2016 at 12:37 PM Ravi <rmi...@chromium.org> wrote:

On Friday, April 8, 2016 at 5:29:39 AM UTC-4, Paweł Hajdan, Jr. wrote:

+infra-dev again

On Fri, Apr 8, 2016 at 11:28 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

+rmistry

Ravi, didn't you add this feature as part of https://bugs.chromium.org/p/chromium/issues/detail?id=504362 ?

Yes, the feature was launched around 10 months ago. Right after launch there were similar complains which is why I had added two ways to skip the dependency checks (they are detailed in the design doc here).

One way is to skip checks for specific branches with "git config branch.test.skip-deps-uploads True" and the other way (possibility more relevant to this thread) is to use "NO_DEPENDENCY_CHECKS=true" in the CL description.

I personally do not think we should add more UI elements to Rietveld for this similar to how we do not have them for the other CQ keywords.

From upthread, I guess that these options are not listed in the error message. Why not just add something like:

"If you are sure that there is no real dependency, you can do XXX to land the CL."

That is a good idea. I will work on improving the message: crbug/601751.