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

Mailer: fix missing getter for smtpHost property #42

Merged
merged 1 commit into from Oct 19, 2018

Conversation

Projects
None yet
9 participants
@metux
Copy link
Contributor

metux commented Sep 7, 2018

required for configuration-as-code

Signed-off-by: Enrico Weigelt, metux IT consult info@metux.net

Mailer: fix missing getter for smtpHost property
required for configuration-as-code

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
@casz

casz approved these changes Sep 7, 2018

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Sep 7, 2018

@casz

This comment has been minimized.

Copy link
Member

casz commented Sep 7, 2018

Silly oleg 🙄 there is one, yet linked in the jcasc github issue: jenkinsci/configuration-as-code-plugin#501
https://issues.jenkins-ci.org/browse/JENKINS-53467
Here ya go 👍

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Sep 8, 2018

Thanks. I believe this is a small quick-win patch we should merge. #39 looks like a bigger change from @ndeloof

@jglick @andresrc do you maintain the plugin? Or is it a shared ownership across the core team?

@metux

This comment has been minimized.

Copy link
Contributor Author

metux commented Sep 8, 2018

By the way: the Delphi language has an own concept for such things, called 'properties'. Looks like dumb fields to the outside, but one can attach getter/setter code if needed.

Could we do something similar in Java ?
Problems like this seem to be pretty common (eg. I've just got a report on the same problem w/ the extended mailer plugin).

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Sep 8, 2018

@ndeloof

This comment has been minimized.

Copy link
Member

ndeloof commented Sep 8, 2018

IIRC it has first been proposed for ... Java 6!

@metux

This comment has been minimized.

Copy link
Contributor Author

metux commented Sep 10, 2018

@ndeloof

This comment has been minimized.

Copy link
Member

ndeloof commented Sep 10, 2018

getter + setter is a very common Java construct, long debated as boilerplate and sometime generated (see Lombok)
There's many things to modernize in Jenkins, and getter+setter "issue" is the last one on my list ;)

@micw

This comment has been minimized.

Copy link

micw commented Oct 5, 2018

Hi. I'm also looking for this fix to use configuration-as-code for automation. Is there anything that prevents this PR to be merged? If not, please do it.
Thank you very much,
Michael.

@jvz

jvz approved these changes Oct 5, 2018

Copy link
Member

jvz left a comment

LGTM.

@micw

This comment has been minimized.

Copy link

micw commented Oct 8, 2018

So, now we have 3 approvals, a Jira issue and a long discussion. What else is required to have that missing getter added?

Kind regards,
Michael.

@ndeloof

This comment has been minimized.

Copy link
Member

ndeloof commented Oct 8, 2018

I guess : someone to actively maintain this plugin :)

@casz

This comment has been minimized.

Copy link
Member

casz commented Oct 8, 2018

well @daniel-beck and @jglick released last, and @oleg-nenashev appears to have commit access 😉

@daniel-beck

This comment has been minimized.

Copy link
Member

daniel-beck commented Oct 8, 2018

Both releases were for security fixes, this does not indicate maintainership.

@micw

This comment has been minimized.

Copy link

micw commented Oct 8, 2018

@daniel-beck Does this mean, that the plug-in is not maintained?
Could you have a look into that minimal PR and maybe merge/release it? It just adds one missing getter, so that plug-in setup can be automated using the configuration-as-code plug-in. It's reviewed and approved by 3 members.
Thank you very much,
Michael.

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Oct 12, 2018

@jglick I believe you are formally the maintainer then. Would you be fine if I also formally join the maintenance team so that I deliver the stuff?

@jglick

This comment has been minimized.

Copy link
Member

jglick commented Oct 12, 2018

I am not a maintainer. If you would like to take over, go ahead.

@ndeloof

This comment has been minimized.

Copy link
Member

ndeloof commented Oct 12, 2018

@olivergondza seems you're maintainer on mailer plugin, can you please check this ?

(and maybe have a look at #39 for a larger scope fix for casc-compatibility)

@oleg-nenashev

This comment has been minimized.

Copy link
Member

oleg-nenashev commented Oct 12, 2018

Let's just call Jenkins core team as maintainers. Will do the job

@oleg-nenashev

This comment has been minimized.

@oleg-nenashev oleg-nenashev referenced this pull request Oct 15, 2018

Closed

Add @oleg-nenashev to the list of maintainers #892

4 of 9 tasks complete

@oleg-nenashev oleg-nenashev merged commit c720f5f into jenkinsci:master Oct 19, 2018

1 check passed

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

This comment has been minimized.

Copy link

micw commented Oct 22, 2018

@oleg-nenashev Thank you very much!

1 similar comment
@micw

This comment has been minimized.

Copy link

micw commented Oct 22, 2018

@oleg-nenashev Thank you very much!

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.