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

introduce hudson.Util#fixNull(value, defaultValue) #3656

Merged
merged 2 commits into from Sep 30, 2018

Conversation

4 participants
@ndeloof
Member

ndeloof commented Sep 28, 2018

  • internal: introduce fixNull(value, defaultValue)
  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs
introduce hudson.Util#fixNull(value, defaultValue)
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@oleg-nenashev

Works for me though it may be already supported by existing libraries bundled into Jenkins

/**
* Convert null to defaultValue.
*/

This comment has been minimized.

@oleg-nenashev

oleg-nenashev Sep 28, 2018

Member

@since TODO

@oleg-nenashev

oleg-nenashev Sep 28, 2018

Member

@since TODO

@batmat

Thanks for adding these.
I'm aware it's trivial code, but still, adding associated tests looks important to me: it's core code that could will likely end up being used dozens or hundreds of times. For usual reasons, having tests for this is important if not critical/required IMO.

@batmat

This comment has been minimized.

Show comment
Hide comment
@batmat

batmat Sep 28, 2018

Member

may be already supported by existing libraries bundled into Jenkins

org.apache.commons.lang.ObjectUtils#defaultIfNull or org.apache.commons.lang3.ObjectUtils#defaultIfNull ?

Member

batmat commented Sep 28, 2018

may be already supported by existing libraries bundled into Jenkins

org.apache.commons.lang.ObjectUtils#defaultIfNull or org.apache.commons.lang3.ObjectUtils#defaultIfNull ?

@Wadeck

Wadeck approved these changes Sep 30, 2018

Note for the code around: it's weird that we are returning unmodifiable collection without noting that on the JavaDoc

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev Sep 30, 2018

Member

Addressed Javadoc/annotation comments for the new method.
@Wadeck could you please create a follow-up newbie-friendly ticket for collections?

Member

oleg-nenashev commented Sep 30, 2018

Addressed Javadoc/annotation comments for the new method.
@Wadeck could you please create a follow-up newbie-friendly ticket for collections?

@Wadeck

This comment has been minimized.

Show comment
Hide comment
Contributor

Wadeck commented Sep 30, 2018

@oleg-nenashev oleg-nenashev merged commit 2fe2ed1 into jenkinsci:master Sep 30, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment