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-71789] Enable ${variable} support for email and cc fields #198

Merged
merged 4 commits into from
Aug 14, 2023

Conversation

mikecirioli
Copy link
Contributor

@mikecirioli mikecirioli commented Aug 9, 2023

See JENKINS-71789 for reproducer and details

Advisor had default support for escaping ${VAR} expressions in the email configuration field, but otherwise did not support resolving variables for email or cc values, and would not correctly encode ${VAR} as {^${VAR} if used as a CC value.

This PR enables encode/decode support for the email and cc values fields.

Testing done

Submitter checklist

Preview Give feedback

@mikecirioli mikecirioli reopened this Aug 11, 2023
@mikecirioli
Copy link
Contributor Author

ping @aheritier for review please!

aheritier
aheritier previously approved these changes Aug 11, 2023
Copy link
Member

@aheritier aheritier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks @mikecirioli

@aheritier
Copy link
Member

please use mvn spotless:apply to fix the formatting @mikecirioli

Copy link
Member

@aheritier aheritier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Thanks a lot !!!

@PierreBtz PierreBtz added the bug label Aug 14, 2023
Copy link
Contributor

@PierreBtz PierreBtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm thanks

@PierreBtz PierreBtz merged commit 58972d1 into jenkinsci:master Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants