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

[JENKINS-50251] JEP-200 failure to serialize Notifier #115

Merged

Conversation

Projects
None yet
3 participants
@jglick
Copy link
Member

commented Mar 19, 2018

JENKINS-50251: used when <ciManagement> specifies email configuration. Follows up #112 & jenkinsci/lib-jenkins-maven-embedder#15.

@reviewbybees

[JENKINS-50251] JEP-200 failure to serialize Notifier, used when <ciM…
…anagement> specifies email configuration.

@jglick jglick requested review from aheritier and oleg-nenashev Mar 19, 2018

@oleg-nenashev
Copy link
Member

left a comment

🐝

@@ -112,7 +113,7 @@
*/
private final String artifactId;

public final Notifier mailNotifier;
public final @CheckForNull NotifierInfo mailNotifier;

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev Mar 19, 2018

Member

May break binary compatibility (detached code), but I do not see external usages

This comment has been minimized.

Copy link
@jglick

jglick Mar 19, 2018

Author Member

The owning class is package-private so it should be safe.

@aheritier
Copy link
Member

left a comment

LGTM @jglick Thx a lot 🐝

@aheritier aheritier merged commit 27ed5b9 into jenkinsci:master Mar 20, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
@aheritier

This comment has been minimized.

Copy link
Member

commented Mar 20, 2018

To release as 3.1.1 I think

@jglick jglick deleted the jglick:ciManagementNotification-JENKINS-50251 branch Mar 21, 2018

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.