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-27363] - Annotate AbstractBuild::getSensitiveBuildVariables() #1601

Closed
wants to merge 1 commit into from

Conversation

@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 11, 2015

AbstractBuild::getSensitiveBuildVariables() may return null from implementations child classes

  • There's no Javadoc in the core to define the correct behavior => plugins behave "correctly"
  • All calls of AbstractBuild::getSensitiveBuildVariables() in the core process null values

Noting is not required.

Signed-off-by: Oleg Nenashev o.v.nenashev@gmail.com

AbstractBuild::getSensitiveBuildVariables() may return null from implementations child classes
* There's no Javadoc in the core to define the correct behavior => plugins behave "correctly"
* All calls of AbstractBuild::getSensitiveBuildVariables() in the core process null values

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
oleg-nenashev added a commit to oleg-nenashev/envinject-lib that referenced this pull request Mar 11, 2015
Patch to the core: jenkinsci/jenkins#1601

Signed-off-by: Oleg Nenashev <o.v.nenashev@gmail.com>
@jglick

This comment has been minimized.

Copy link
Member

jglick commented Mar 30, 2015

There's no Javadoc in the core to define the correct behavior

Unless Javadoc explicitly claims that null is allowed, it should be considered forbidden.

Anyway there are few subclasses of AbstractBuild so it should be straightforward to verify that none of them override this method to return null.

@oleg-nenashev

This comment has been minimized.

Copy link
Member Author

oleg-nenashev commented Apr 3, 2015

@jglick
The approach is correct according to Sun/Oracle code conventions, but the reports to EnvInject plugin indicate that one of subclasses does it. See logs in https://issues.jenkins-ci.org/browse/JENKINS-23447. I have not found the cause yet (FreeStyleBuild from logs cannot be responsible). GitHub search
is not very helpful + ore extension points list is missing on Wiki, but I hope I checked the most popular plugins.

It's OK for me to leave the method @nonnull

@jglick

This comment has been minimized.

Copy link
Member

jglick commented Apr 4, 2015

The null in that case was from EnvInjectAction.getSensibleVariables() [sic], not AbstractBuild.

@oleg-nenashev

This comment has been minimized.

Copy link
Member Author

oleg-nenashev commented Aug 20, 2016

The PR is obsolete

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