Skip to content

Fixing NullPointerExceptions in PullRequests #245

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

Merged
merged 1 commit into from
Nov 30, 2019

Conversation

2improveIT
Copy link
Contributor

Fixing NullPointerExceptions

Your checklist for this pull request

  • 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.

@jetersen
Copy link
Member

Thanks for resubmitting this PR 😊

@jetersen jetersen closed this Nov 5, 2019
@jetersen jetersen reopened this Nov 5, 2019
@jetersen
Copy link
Member

jetersen commented Nov 5, 2019

link to see current commit author: https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/245/commits/710faec6e6072648bc641618ca1908bfa40511e1.patch

@2improveIT
You might want to set up an email alias on your GitHub account 🥇
https://help.github.com/articles/adding-an-email-address-to-your-github-account/

Or look into fixing your git user.email config 😅
https://help.github.com/articles/setting-your-commit-email-address-in-git
then you would need to amend your commits: https://stackoverflow.com/a/3042512
after changing author

to get proper credit for your commits 🏆

Copy link
Contributor

@darxriggs darxriggs left a comment

Choose a reason for hiding this comment

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

Could you provide some details in the description under which circumstances you have seen the NPEs occur?

if (branch != null) {
branch.setCommitClosure(new CommitClosure(branch.getRawNode()));
}
} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really want to catch ALL exceptions here?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, I missed that 😱

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adapted the code

branch.setCommitClosure(new CommitClosure(branch.getRawNode()));
}
} catch (Exception e) {
LOGGER.log( Level.SEVERE, "setupClosureForPRBranch", e);
Copy link
Contributor

@darxriggs darxriggs Nov 9, 2019

Choose a reason for hiding this comment

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

The formatting could be improved by removing the trailing space after LOGGER.log( .

Suggested change
LOGGER.log( Level.SEVERE, "setupClosureForPRBranch", e);
LOGGER.log(Level.SEVERE, "setupClosureForPRBranch", e);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adapted the code

@jetersen jetersen merged commit e8c8a3c into jenkinsci:master Nov 30, 2019
@jetersen jetersen added the bug label Jan 28, 2020
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