New issue
Advanced search Search tips
Starred by 6 users

Issue metadata

Status: Submitted
Owner: ----
Closed: Apr 25
Cc:
Components:



Sign in to add a comment

the status of an wip change is shown as merge conflict in old ui

Reported by cdlee...@gmail.com, Mar 27

Issue description

Affected Version: 2.15-rc4

What steps will reproduce the problem?
1. set a ready to submit change to wip.

What is the expected output?
the status of change should be wip in old ui.

What do you see instead?
merge conflict.

Please provide any additional information below.

 
screenshot-2.png
52.2 KB View Download
Project Member

Comment 1 by jrn@google.com, Mar 27

Components: -PolyGerrit GWT
Labels: Hotlist-WIP
This sounds a bit like  issue 8381 .

Project Member

Comment 3 by dborowitz@google.com, Mar 27

IMHO as long as it says "work in progress" there then the user should be able to figure out that they need to mark it not-WIP in order to submit. I don't see this as a blocker for 2.15.
If that was my change, and I was ready to submit, the first thing I'd see would be "Merge Conflict".

Without that warning, yes, I'd just take off the WIP.

But seeing the "Merge Conflict", I'd leave it WIP while I tried to fix the merge conflict locally and push up a new patchset, and then be puzzled when I couldn't find the conflict.

This is highly confusing, at best, though it might not be a blocker.

I guess we could release 2.15 now, and fix this one up in 2.15.1. Given the number of changes in 2.15, I image it's inevitable we'll need a few other fixes anyhow.

Due to the time it's taken to get 2.15 release-ready (for very good reasons - this isn't a complaint), master (and in particular PolyGerrit) has moved on a lot, so I suspect we're looking at a 2.16 release soonish.
Project Member

Comment 5 by dborowitz@google.com, Mar 28

> I guess we could release 2.15 now, and fix this one up in 2.15.1. Given the number of changes in 2.15, I image it's inevitable we'll need a few other fixes anyhow.

Yeah, we've gotta draw the line somewhere, and I'm drawing it here.
Fair enough. Let's do it!
would this issue be fixed in 2.15.x?
To recap: in 2.15, a "WIP" change displays as "Merge Conflict" in the old UI.

The bug is still there in 2.15.1, and I haven't seen any fix for it. I suspect that no-one is working on it. I'm a bit surprised that more folks haven't reported this.

According to the latest discussion on the mailing list [1] the issue is not occurring on the version running at gerrit-review, which is a recent build off master.

It might have got fixed on master since 2.15 was released, but I've just had a look through and couldn't find anything related.

[1] https://groups.google.com/d/msg/repo-discuss/-jTFUIwLyHc/aY3WhgI_AQAJ
Project Member

Comment 10 by david.os...@gmail.com, Apr 24

Status: Accepted (was: New)
It is not fixed on master.

The reason for that is because, change info entity doesn't
have mergeable attribute set for WIP changes:

https://github.com/GerritCodeReview/gerrit/blob/master/java/com/google/gerrit/server/query/change/ChangeData.java#L991-L993

And GWT based change screen requires mergeable flag to be set
to render "Ready to submit" status textm abd renders merge
conflict otherwise:

https://github.com/GerritCodeReview/gerrit/blob/master/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.java#L1262-L1265




Thanks for the research.

>> It is not fixed on master.

Interesting. I thought it had been fixed there, simply because at https://gerrit-review.googlesource.com/, I don't see the "Merge Conflict" in the old UI for WIP changes.

So why don't I see the problem there? I guess that particular Gerrit has been heavily customised, so we can't really draw any conclusions about the open source release.
Project Member

Comment 12 by david.os...@gmail.com, Apr 24

> I thought it had been fixed there, simply because at https://gerrit-review.googlesource.com/, I don't see the "Merge Conflict" in the old UI for WIP changes.

Google has *not* customized GWT change screen. You are
missing one subtle fact here: 

until a change is ready to submit, you would see a
different status text, like "Blocked on foo" or
"Needs bar". IOW to reproduce the problem reported
here, a change must be ready to submit to enter this
if statement branch (see my previous comment why):

    if (canSubmit && status == Change.Status.NEW) {
      statusText.setInnerText(
            changeInfo.mergeable() ? Util.C.readyToSubmit() : Util.C.mergeConflict());
    }

That why you need a ready to submit change in WIP state.
There is no such a change currently on gerrit review:

You could proof this with this query, that returns no
result:

https://gerrit-review.googlesource.com/#/q/is:wip+status:open+label:Code-Review%252B2+-label:Code-Review-2

However, on another tenant: go-review, there are two changes
that satisfy our condition:

https://go-review.googlesource.com/?polygerrit=0#/c/go/+/33637
https://go-review.googlesource.com/?polygerrit=0#/c/go/+/36355

Et voilá:

both changes do have status "Merge Conflict (WorkInProgress)"
Status: ChangeUnderReview (was: Accepted)
Thanks David.  I was looking at the code based on the assumption that "it's fixed in master" was correct.



https://gerrit-review.googlesource.com/#/c/gerrit/+/174072
This is what it looks like now:

https://imgur.com/a/cgkoARM

The change list also suffers a similar problem, which I have fixed in a separate change still referencing this issue:

https://gerrit-review.googlesource.com/#/c/gerrit/+/174150

Labels: FixedIn-2.15.2
Status: Submitted (was: ChangeUnderReview)
Thanks for the explanation and fix.

Sign in to add a comment