Comment 1 Sam Davis 2014-02-25 12:25:54 EST

Thanks for reporting this. We should try to fix it and consider cherry-picking the fix for 3.11.

Comment 2 Tomasz Zarna 2014-02-27 03:36:38 EST

Christian, is your Gerrit configured to use any custom approvals? Other than Verified, Code-Review?

Comment 3 Christian Pontesegger 2014-02-27 14:06:11 EST

(In reply to Tomasz Zarna from comment #2)
> Christian, is your Gerrit configured to use any custom approvals? Other than
> Verified, Code-Review?

No, just the two of them

Comment 4 Tomasz Zarna 2014-03-03 05:16:54 EST

Hmm, in that case I don't know why you're getting the error. To move this forward I pushed https://git.eclipse.org/r/#/c/22757/ which logs more details when the exception happens. It's not a fix, but it should help finding the culprit.

Comment 5 Tomasz Zarna 2014-03-03 05:16:59 EST

Created attachment 240449 [details]
mylyn/context/zip

Comment 6 Christian Pontesegger 2014-03-12 14:55:31 EDT

(In reply to Tomasz Zarna from comment #4)
> Hmm, in that case I don't know why you're getting the error. To move this
> forward I pushed https://git.eclipse.org/r/#/c/22757/ which logs more
> details when the exception happens. It's not a fix, but it should help
> finding the culprit.

Is this part of some weekly build I could try out on my setup?

Comment 7 Tomasz Zarna 2014-03-12 16:05:57 EDT

Not sure about the weekly update site [1], but the one with nightlies [2] should already have the logging patch. If you could give it a spin and let me know what the Error Log says it would be great.

[1] http://download.eclipse.org/mylyn/snapshots/weekly
[2] http://download.eclipse.org/mylyn/snapshots/nightly

Comment 8 Christian Pontesegger 2014-03-24 10:09:46 EDT

I tried with the nightly update site. Additionally to the reported exception I see following log entry:

java.lang.NullPointerException
	at org.eclipse.mylyn.internal.gerrit.ui.operations.GerritOperationDialog.performOperation(GerritOperationDialog.java:131)
	at org.eclipse.mylyn.internal.gerrit.ui.operations.GerritOperationDialog.close(GerritOperationDialog.java:84)
	at org.eclipse.jface.dialogs.Dialog.okPressed(Dialog.java:955)
	at org.eclipse.jface.dialogs.Dialog.buttonPressed(Dialog.java:476)
	at org.eclipse.mylyn.reviews.ui.ProgressDialog.access$1(ProgressDialog.java:1)
	at org.eclipse.mylyn.reviews.ui.ProgressDialog$2.widgetSelected(ProgressDialog.java:265)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1057)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4170)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3759)
	at org.eclipse.jface.window.Window.runEventLoop(Window.java:826)
	at org.eclipse.jface.window.Window.open(Window.java:802)
	at org.eclipse.mylyn.internal.gerrit.ui.operations.GerritOperationDialog.open(GerritOperationDialog.java:218)
	at org.eclipse.mylyn.internal.gerrit.ui.factories.PublishUiFactory.execute(PublishUiFactory.java:49)
	at org.eclipse.mylyn.reviews.ui.spi.factories.AbstractUiFactory.handleExecute(AbstractUiFactory.java:81)
	at org.eclipse.mylyn.reviews.ui.spi.factories.AbstractUiFactory.access$0(AbstractUiFactory.java:79)
	at org.eclipse.mylyn.reviews.ui.spi.factories.AbstractUiFactory$1.widgetSelected(AbstractUiFactory.java:72)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1057)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4170)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3759)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1113)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:997)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:138)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:610)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:567)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:354)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:181)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:636)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:591)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1450)

Comment 9 Tomasz Zarna 2014-03-25 10:59:31 EDT

(In reply to comment #8)
> Additionally to the reported exception I see following log entry:

Are you saying you still get the NPE from comment 0? I would expect it to be gone and you should see a warning like this "Couldn't find label name for .... (Expected approval IDs are [IPCL, CRVW, VRIF])". Are you sure you're on a nightly? Can you check (and paste in) the exact version you're using?

Comment 10 Christian Pontesegger 2014-03-26 11:18:52 EDT

My fault. Seems the upgrade path from weekly builds to nightly builds is broken.
On a fresh Eclipse RCP with the nightly build N20140312 the error is gone.

I could successfully review and give +2.
When trying to Verify and set +1 this was simply ignored. No comment wad created, verify is not set ("? Verify" displayed).
Should I open a new bug for this?

Comment 11 Tomasz Zarna 2014-03-26 11:43:22 EDT

(In reply to Christian Pontesegger from comment #10)
> My fault. 

No worries.
 
> When trying to Verify and set +1 this was simply ignored. No comment wad
> created, verify is not set ("? Verify" displayed).
> Should I open a new bug for this?

Please do, this way we can mark this as fixed. btw, after filing the new bug, could you add there any logs you think can be related (if any available). Thx!

Comment 12 Sam Davis 2014-03-26 12:43:42 EDT

It sounds to me like the issue here is that for some reason the Verify approval uses a different key than expected by the connector. At any rate I don't see the need to open a separate bug as it's probably the same cause as the NPE. The logging that Tomasz added should help to identify the cause.

Comment 13 Tomasz Zarna 2014-03-26 14:03:21 EDT

Right, Christian have you found any log entries similar to the one mentioned in comment 9?

Comment 14 Christian Pontesegger 2014-04-04 05:30:27 EDT

I jst tried. Now when I use "Publish Comments", I give my +2 and hit OK. There I get a warning in the error log:

Couldn't find label name for null. (Expected approval IDs are [IPCL, CRVW, VRIF])
No stacktrace attached

Still publishing comments works.
The only other topic is the Verify visualization, which looks odd (but this is just a graphical matter). Screenshot follows

Comment 15 Christian Pontesegger 2014-04-04 05:33:03 EDT

Created attachment 241604 [details]
Review screenshot

I can see the +2 next to my name, but there is no +1 next to the Jenkins user. Still the verify checkmark in the header row is set correctly.

Comment 16 Tomasz Zarna 2014-04-07 19:00:07 EDT

Thanks for giving it a try Christian. Unfortunately the warning doesn't gives us much info, but it looks like there is something fishy about the Verify approval (which I guess is missing for Jenkins in your screenshot). Is there any chance I can connect to the Gerrit instance you're using? Is it publicly available?

Comment 17 Christian Pontesegger 2014-04-08 03:45:40 EDT

sorry, this is an internal server which cannot be accessed from the outside.

As you just noted it has to do with the verify. Just tested and I am not able to set verify to +1/-1. Only thing I get is the warning reported in comment 14.

Comment 18 Sam Davis 2014-04-29 20:05:03 EDT

*** Bug 430389 has been marked as a duplicate of this bug. ***

Comment 19 Sam Davis 2014-07-31 16:01:27 EDT

Tomek, I took a quick look at the code and it seems like the approvals are hard coded and it's not obvious that custom approvals work at all. Do you know if the connector actually works with custom approvals?

Comment 20 Tomasz Zarna 2014-07-31 18:10:46 EDT

Just to be clear, they have been hard-coded from day 1. What I did, while working on the connector, was moving them all in one class so it's easier to remove :)

As for custom approvals, I'm not sure, but my guess would be we tried to not get in the way. Meaning, do not panic when seeing one.

Comment 21 Sam Davis 2014-07-31 18:49:36 EDT

Thanks. I'm moving this to the backlog since there's been no activity for a while. Feel free to move it back to 3.13 if you think you can fix it.

Comment 22 Oleksandr Presich 2014-11-11 10:11:55 EST

Hello there!

Guys, are there any plans to provide support for custom flags?

We are using them heavily so it will be very useful feature for us!

Comment 23 Sam Davis 2014-11-12 13:01:04 EST

We have no immediate plans to work on this, but I'm marking it helpwanted to indicate that we would be happy to support a contribution.

Comment 24 Eclipse Webmaster 2022-11-15 11:45:08 EST

Mylyn has been restructured, and our issue tracking has moved to GitHub [1].

We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub.

[1] https://github.com/orgs/eclipse-mylyn