Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update POMs and cleanup the dependencies/code #192

Merged
merged 1 commit into from
Feb 7, 2018

Conversation

oleg-nenashev
Copy link
Member

I was working on PCT for JEP-200, and finally I discovered that the plugin build does not work there. In this PR i tried to cleanup the POMs and code a bit.

  • Core dependency is bumped to the version required by current Pipeline dependencies
  • Use Parent POM 3.2
  • Get rid of explicit Core/War dependencies in TFS Plugin. They were suppressing some analysis, and generally it's not required since parent POM inherits from Plugin POM
  • Fix compilation failures in the code. For me it was failing even in the stock version, but please double-check

@reviewbybees

@ghost
Copy link

ghost commented Jan 12, 2018

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@@ -111,7 +111,8 @@ private String getUserId() {
final String userName = getSystemProperty(SYS_PROP_USER_NAME);
final String fakeUserId = MessageFormat.format("{0}@{1}", userName, computerName);

return DigestUtils.sha1Hex(fakeUserId);
//FIXME: was sha1Hex, but this is available only in commons codec 1.7. TFS SDK 14.0.3 bundles older version
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this needs to be fixed (i.e. you can remove the comment). The idea here is to keep the user id unique to the telemetry server, but not allow anyone to actually identify the user. So, as long as the user id is obfuscated, there shouldn't be a problem.

Copy link
Contributor

@jpricket jpricket left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes. I was trying to do this myself just yesterday, but didn't get it all working.


<properties>
<!-- TODO: enable once FindBugs issues are fixed -->
<findbugs.failOnError>false</findbugs.failOnError>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? I hesitate to turn it off.

Copy link
Member Author

Choose a reason for hiding this comment

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

With the current configuration you will be getting FindBugs warnings in the build log. They just won't fail the build.

@jpricket jpricket merged commit da8a2ae into jenkinsci:master Feb 7, 2018
keljos added a commit that referenced this pull request Mar 22, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants