Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix eclipse warnings #242

Merged
merged 6 commits into from Sep 1, 2015

Conversation

Projects
None yet
3 participants
@dpursehouse
Copy link
Contributor

commented Sep 1, 2015

No description provided.

dpursehouse added some commits Sep 1, 2015

Fix various minor code style issues
These are showing up as errors when opening the project in
Eclipse after generating the Eclipse project with:

 mvn eclipse:eclipse

Change-Id: Ic7c6b73b36aad72678d03d4f2d19c2758cebb184
Remove deprecated and unused allowTriggeringUnreviewedPatches
Change-Id: Ibaea371794a0666566de1860df3a63863fbafbca
GerritServer: remove unused local variable
Change-Id: Ie96a248dd93cea9b002b6468da169c82548e8ed8
GerritTriggerTest: remove unused assertParamEquals method
Change-Id: I5563d06b22fa5d0237b2264076b02d6ac75ad55a
@jenkinsadmin

This comment has been minimized.

Copy link
Member

commented Sep 1, 2015

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@@ -172,8 +172,6 @@
private String serverName;
private String gerritSlaveId;
private List<PluginGerritEvent> triggerOnEvents;
@Deprecated
private transient boolean allowTriggeringUnreviewedPatches;

This comment has been minimized.

Copy link
@rsandell

rsandell Sep 1, 2015

Member

👎 This will break loading of old configurations.

This comment has been minimized.

Copy link
@dpursehouse

dpursehouse Sep 1, 2015

Author Contributor

Fixed in the follow up commit cb3c5b5.

@dpursehouse

This comment has been minimized.

This is the line that was causing a warning. The allowTriggeringUnreviewedPatches on the right hand side of the assignment is not a constructor argument like the others, so it resolves to the class member and is just assigning the same member variable to itself.

Put back allowTriggeringUnreviewedPatches
As mentioned by @rsandell in review, removing the member variable
in commit d90eb78 will break loading of old configs.

Put it back, and instead of assigning it to itself, just assign
to false.

Change-Id: I7c9ea89dd7675588bf2bcee464b7a0c635df7bc2
@@ -302,7 +302,7 @@ public GerritTrigger(
this.triggerConfigURL = triggerConfigURL;
this.gerritTriggerTimerTask = null;
triggerInformationAction = new GerritTriggerInformationAction();
this.allowTriggeringUnreviewedPatches = allowTriggeringUnreviewedPatches;
this.allowTriggeringUnreviewedPatches = false;

This comment has been minimized.

Copy link
@rsandell

rsandell Sep 1, 2015

Member

just remove this line completely and put @SuppressWarnings("unused") on the declaration instead.

This comment has been minimized.

Copy link
@dpursehouse

dpursehouse Sep 1, 2015

Author Contributor

Done with 32eb411

Remove initialisation of allowTriggeringUnreviewedPatches
and suppress the warning about it being unused.

Change-Id: Iba6b41c6759c2bef73e626dd09e3efedc8628192

rsandell added a commit that referenced this pull request Sep 1, 2015

@rsandell rsandell merged commit b277176 into jenkinsci:master Sep 1, 2015

1 check passed

Jenkins This pull request looks good
Details

@dpursehouse dpursehouse deleted the dpursehouse:fix-eclipse-warnings branch Sep 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.