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

Prevent null pointer when exporting file credentials #1282

Merged
merged 7 commits into from
Feb 27, 2020

Conversation

timja
Copy link
Member

@timja timja commented Feb 22, 2020

Requires jenkinsci/credentials-plugin#135
Fixes #1170

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

@timja timja added the bugfix A PR that fixes a bug - used by Release Drafter label Feb 22, 2020
@timja timja requested a review from a team as a code owner February 22, 2020 09:58
Object value = a.getValue(instance);
if (value != null) {
Object converted = Stapler.CONVERT_UTILS.convert(value, a.getType());
if (converted instanceof Collection || p.getType().isArray() || !a
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be formatted differently it looks odd.

Copy link
Member

@jetersen jetersen left a comment

Choose a reason for hiding this comment

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

LGTM once you format the if statement

@jetersen
Copy link
Member

Build fails due to upper bound deps.

@timja
Copy link
Member Author

timja commented Feb 22, 2020

sorted

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Sounds about right 👍

@codecov
Copy link

codecov bot commented Feb 22, 2020

Codecov Report

Merging #1282 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1282   +/-   ##
=========================================
  Coverage     78.83%   78.83%           
  Complexity      780      780           
=========================================
  Files            65       65           
  Lines          2301     2301           
  Branches        320      320           
=========================================
  Hits           1814     1814           
  Misses          386      386           
  Partials        101      101

@timja timja closed this Feb 24, 2020
@timja timja reopened this Feb 24, 2020
@timja
Copy link
Member Author

timja commented Feb 24, 2020

(this previously passed, re-triggering)

@timja timja closed this Feb 24, 2020
@timja timja reopened this Feb 24, 2020
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

ready to go IMHO

@timja
Copy link
Member Author

timja commented Feb 27, 2020

I've been waiting for jenkinsci/credentials-plugin#135 to be merged and released as we're currently depending on an incremental version in the integration tests

cc @jvz

@jvz
Copy link
Member

jvz commented Feb 27, 2020

I've published credentials 2.3.2 @timja

@jetersen
Copy link
Member

jetersen commented Feb 27, 2020

@jvz I think you forgot to merge the PR: jenkinsci/credentials-plugin#135 😱

@jvz
Copy link
Member

jvz commented Feb 27, 2020

Womp womp. Publishing 2.3.3 right now as well as an updated changelog in it.

@timja timja merged commit 31595ec into jenkinsci:master Feb 27, 2020
@jetersen
Copy link
Member

👏

@@ -67,6 +67,31 @@ public void testDomainScopedCredentials() {
assertEquals("secret", creds.get(0).getPassword().getPlainText());
}

@Test
Copy link
Member

Choose a reason for hiding this comment

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

Add

@Issue("JENKINS-60467")

IIUC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix A PR that fixes a bug - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible NPE when DataBound Object attribute is not Describable
5 participants