Skip to content

Conversation

@ldaley
Copy link
Contributor

@ldaley ldaley commented Jan 21, 2014

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.

ldaley added 24 commits January 20, 2014 12:47
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
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.
@cloudbees-pull-request-builder

RxJava-pull-requests #684 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

RxJava-pull-requests #685 FAILURE
Looks like there's a problem with this pull request

@ldaley
Copy link
Contributor Author

ldaley commented Jan 21, 2014

I don't have an explanation for this latest failure.

It's passing locally.

@akarnokd
Copy link
Member

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.

@ldaley
Copy link
Contributor Author

ldaley commented Jan 21, 2014

Is there anyway to retrigger the check here so we can verify the build passes with this PR?

Copy link
Contributor

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.

Copy link
Contributor Author

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…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cloudbees-pull-request-builder

RxJava-pull-requests #686 FAILURE
Looks like there's a problem with this pull request

@benjchristensen
Copy link
Member

@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?

@ldaley
Copy link
Contributor Author

ldaley commented Jan 22, 2014

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.

@benjchristensen
Copy link
Member

Got it, thanks @alkemist

@quidryan
Copy link
Collaborator

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.

@ldaley
Copy link
Contributor Author

ldaley commented Jan 23, 2014

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?

@benjchristensen
Copy link
Member

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

@ldaley
Copy link
Contributor Author

ldaley commented Feb 11, 2014

Sure thing.

Will take a few days.

@benjchristensen
Copy link
Member

Thanks @alkemist I'll close this one out for now.

jihoonson pushed a commit to jihoonson/RxJava that referenced this pull request Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants