-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Various buildscript cleanup. #772
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
Conversation
The value being specified is the conventional default.
- Make extra 'examples' and 'perf' source sets super sets of test classpaths (simplifies IDE configuration) - Consolidate all standard configuration to convention.gradle - Remove redundant configuration - Use more conventional configuration idioms
… the main compile classpath.
Gets test dependencies off the compile classpath and removes the need to prevent imports of packages in the OSGi manifest.
This allows them to be properly shared and tested, and this configuration maps to IDEs better.
No functional change.
No functional change.
No functional change.
It's only used at the root.
|
RxJava-pull-requests #684 FAILURE |
|
RxJava-pull-requests #685 FAILURE |
|
I don't have an explanation for this latest failure. It's passing locally. |
|
The test needs to run something on an external thread and is time critical, therefore, if the build machine is bogged down for some reason, it may fail. Unfortunately, I can't fix the test right now. |
|
Is there anyway to retrigger the check here so we can verify the build passes with this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a real dependency of the module so should remain provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, it's used in the examples only, fixing…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added ldaley@f086917
|
RxJava-pull-requests #686 FAILURE |
|
@quidryan Justin, since most of the gradle setup and config is from you I think you need to be involved in this review. @alkemist Thank you for the submission. We also need to validate this doesn't break the internal build process that builds and publishes the code to our internal repo and to Maven Central. What is it that triggered these changes? Were you having problems of some kind? |
|
I just noticed that things were a bit disorderly and thought I could help. No problems per se, but there were issues around classpath management that probably would have reared eventually. Secondary motivation is wanting to ensure that this project is a good example of Gradle usage. |
|
Got it, thanks @alkemist |
|
There's some history to the build. We fork from a common repo called gradle-template, and then merge changes out to the 40-odd netflix projects. It's not ideal and it's old, but it works and is expected to be replaced soon. Changes like whitespace and moving sections around makes it nearly impossible to merge, so I'd ask that the structural bits of the build files be left alone and taken out of the pull request. E.g. the github-pages needs to stay where it was. I can see that there's lot of redundant definitions, which is not in the gradle-template, have crept into this build and those can be consolidated, like the javadoc. |
|
Can you elaborate on what you consider structural changes? Is this just where files are? Or also things like getting the test dependencies off the compile classpath? |
|
@alkemist Many of these changes in this PR would be better suited I think to fixing the parent at https://github.com/Netflix/gradle-template. If they are fixed there we will then pick them up. The second group of changes for RxJava specific build files, I would gladly accept cleanup if you can separate them from those that affect anything from gradle-template. |
|
Sure thing. Will take a few days. |
|
Thanks @alkemist I'll close this one out for now. |
There's a lot of changes here, but I tried to keep them small and incremental.
Let me know if there's anything you're unhappy with and I'll recraft this.