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

Fix customizing pom.xml in Gradle build #582

Merged
merged 4 commits into from
Mar 6, 2018

Conversation

lhotari
Copy link

@lhotari lhotari commented Feb 28, 2018

Fixes #581 and improves the way how pom.xml files are customized .

@msftclas
Copy link

msftclas commented Feb 28, 2018

CLA assistant check
All CLA requirements met.

@dhaval24
Copy link
Contributor

@lhotari could you sign the cla? Thank you very much for your contribution.

@lhotari
Copy link
Author

lhotari commented Feb 28, 2018

@dhaval24 I signed it now.

@dhaval24
Copy link
Contributor

@lhotari thank you very much for your valuable contribution and standing up to help in our critical time. I really appreciate it. Hope you give application insights a try and let us know the experience. We would constantly try to take feedback and improve. Looking forward to see even more contribution in future :) @littleaj will review your build changes and suggest any updates or questions he may have as he primarily manages it! I will glance my eyes through it also.

@dhaval24
Copy link
Contributor

dhaval24 commented Mar 4, 2018

@lhotari briefly looking over this, we do have currently one issue in our POM. It mentions some of the dependencies as guava, httpclient etc that we shade as 1st order compile dependencies. This can lead to potential class path hells. We do not really need them to be mentioned as first order dependencies in the POM as they are shaded. #583 for more details. Does this PR fixes that also?

@lhotari
Copy link
Author

lhotari commented Mar 4, 2018

@dhaval24 I now added a commit to this PR to also fix #583

@dhaval24 dhaval24 added this to the 2.0.2 milestone Mar 4, 2018
Copy link
Contributor

@dhaval24 dhaval24 left a comment

Choose a reason for hiding this comment

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

Some newbie questions on gradle :)

afterEvaluate {
assert (!projectPomName.isEmpty() && !projectPomDescription.isEmpty())
pom {
uploadArchives { task ->
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do these parameters(username, pass, url) get passed through now? I see you trimmed down the the hard coded urls.

Copy link
Author

@lhotari lhotari Mar 5, 2018

Choose a reason for hiding this comment

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

I didn't change the way the parameters are passed. The mavenRepositoryUrl, mavenUsername and mavenUserPassword parameters are assumed to be passed by gradle project properties (more), just like before. To explain the change that you have commented on:
Instead of adding the uploadArchives task in afterEvaluate, I moved the repository config to the doFirst block of uploadArchives task. I also changed the way that the existence of mavenRepositoryUrl, mavenUsername and mavenUserPassword parameters are checked. These parameters are only required when the uploadArchives task is part of the Gradle task graph and will be executed during the build. The same approach was used in Gradle core build.

@littleaj littleaj mentioned this pull request Mar 6, 2018
2 tasks
Copy link
Contributor

@littleaj littleaj left a comment

Choose a reason for hiding this comment

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

Please review the changes I made in #592. I'd like to combine what you did here with the changes I made (specifically, the dependencies filter)

As I said in the comments, we don't use uploadArchives anymore, but we still need the pom as an XML file. If you have ideas regarding how we could better generate the pom file, just @ me on either PR.

Thanks for your work on this! :)

afterEvaluate {
assert (!projectPomName.isEmpty() && !projectPomDescription.isEmpty())
pom {
uploadArchives { task ->
Copy link
Contributor

Choose a reason for hiding this comment

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

We actually don't use the uploadArchives task anymore. We now use an external mechanism.

We do still need the build to generate the poms in the "artifacts directory" (see getArtifactsDirectory). See my latest PR, #592, where I've addressed this.

Copy link
Author

Choose a reason for hiding this comment

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

@littleaj Thanks for the clarification. I added a commit bed9829 to this PR that makes #592 obsolete. That commit adds a task "generatePom" that generates a pom beside the artifact jar file. This task is a now a dependency of copyLibsToGlobalArtifactsFolder task, so it should cover your way to publish.
I think it makes sense to keep "uploadArchives" and "install" tasks since those help users of the library to work with local forks and patches. Users would typically want to start using a change internally before the changes have been merged upstream.

Copy link
Contributor

@littleaj littleaj left a comment

Choose a reason for hiding this comment

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

Great work! Thanks for you contribution

@littleaj littleaj merged commit 40f4e35 into microsoft:2.0.0-STABLE Mar 6, 2018
littleaj added a commit that referenced this pull request Mar 8, 2018
* Fix null ref check in telemetry correlation Utils (#541)

* Fix null ref check in TelemetryCorrelationUtils

* Modifying log level to warning

* Updating Changelog

* Fix handling of NaN and +/-Infinity in JSON serializer (#499)

* Handle NaN and +/-Infinity in metrics

* Default NaN/Infinity serialization to 0 to be consistent with other AI SDKs and make the code compatible with Java 6

* fixed javadoc errors and added section to generate pom.xml with all builds (#551)

* Updating version number to 2.0.0

* Implementing Retry logic [Reliable Channel] [STABLE Branch] (#561)

* Initial commit of retry and backoff logic fixes

* Fixing warnings on files I touched this round

* Fix the eclipse UI from screaming about the docker Contstants

* Fixed backoff logic to use existing method. Added more logging to the sender channel.

* Added the partial response handler, more logging

* Added gson to core. Fixed backoff manager to keep original functionality. Added extension to return the timeout values as expected before.

* Added unit tests.

* Fixing string typed ArrayList<> to List<> per Dhaval

* Missed one

* Making tests consistent.

* Added javadoc comments, simplified logic for a few methods

* Added exception logging per @dhaval24. Fixed formatting on touched files

* Updates per last round of commits

Moved the Handlers out of the concrete package to the common package to keep the same consistency.  Removed a couple of unessecary methods. Added docs.

* Latest fixes

* Add MaxInstantRetry

Added MaxInstantRetry configuration to allow for instantaneous retry on a failed transmission.

* Javadoc Updates

Javadoc and formatting updates

* NumberFormatException fix

Added null check

* JavaDocs for TPM

* Fixing FixedRateSampling to work in old and new version of sampling (#540)

Overriding default sampling percentage when programatically specified sampling percentage by user.

* upgrade to logback v1.2.3 (#565)

* Reliable channel: replacing logAlways "TRACE" statements with "info" (#571)

* Reliable channel: close resources in finally block. (#572)

* Reliable channel: close resources in finally block.

* change logging to warning when closing resources

* Bugfix against retry logic (#576)

* Refactor

* BUGFIX Logic would never backoff

After adding the instant retry amount logic to the code this line of code could cause the transmissions to not back off.

* Changes requested

* Fixed javadocs tags, that caused build errors while executing `javadoc` gradle task (#578)

* Update Changelog

* Fix link in changelog

* Fix another link in changelog

* Update gradle.properties

* Fix customizing pom.xml in Gradle build (#582)

* Fix customizing pom.xml in Gradle build

* Insert license after 1. row in pom.xml

* Filter artifacts relocated by shadow task from pom dependencies

- match artifacts by groupId
- fixes #583 

* Generate a pom file "beside" the artifact jar file
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