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

filenameCollisionCheck defaults to false #470

Merged
merged 1 commit into from
Sep 30, 2015

Conversation

brunobowden
Copy link
Contributor

Defaults to True when translateArgs contains “—no-package-directories”

Fixes #467

@brunobowden
Copy link
Contributor Author

@advayDev1 - PTAL

*/
boolean filenameCollisionCheck = true
boolean filenameCollisionCheck = false
Copy link
Contributor

Choose a reason for hiding this comment

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

would be clearer to call this 'forceFilenameCollisionCheck' and then change the javadoc.
it is not the case that this variable should be set to true, in fact we are just treating it as if it was true when no-package-directories is present.

@advayDev1
Copy link
Contributor

lgtm with comments

Defaults to True when translateArgs contains “—no-package-directories”

Fixes j2objc-contrib#467
@brunobowden
Copy link
Contributor Author

So Clang allows compiles with the directory structure from the command line but not through Xcode?

Just waiting for CI to complete and then I'll merge.

@brunobowden
Copy link
Contributor Author

Merging...
The Xcode build now takes 18 minutes which is a long time.

brunobowden added a commit that referenced this pull request Sep 30, 2015
filenameCollisionCheck defaults to false
@brunobowden brunobowden merged commit 3b48fb7 into j2objc-contrib:master Sep 30, 2015
@advayDev1
Copy link
Contributor

Xcode actually passes every individual file to clang separately. So I think it is xcode's 'choice' not to support directories. Also what is taking 18 minutes? Which task?

@brunobowden
Copy link
Contributor Author

The recent Xcode builds take 18-20 minutes:
https://travis-ci.org/j2objc-contrib/j2objc-gradle/jobs/82959080

Adding the Guava was responsible, you can see that from the build history:
https://travis-ci.org/j2objc-contrib/j2objc-gradle/builds

@advayDev1
Copy link
Contributor

OH: the continuous integration builds are 18 minutes. Not like your dev Xcode builds for your project.

Yeah, that is true. But I don't see how we can really exercise the plugin unless it is building multiple projects with dependencies - Guava's our best bet for our various features really.

@advayDev1
Copy link
Contributor

This is the first time I'm actually comfortable that changes aren't breaking the plugin. Unit-tests can only do so much with the amount of inter-tool interaction we have

@brunobowden
Copy link
Contributor Author

The level of complexity in the system is considerable. It's a lot more
complex that your typical library.

On Wed, Sep 30, 2015 at 9:51 AM Advay Mengle [email protected]
wrote:

This is the first time I'm actually comfortable that changes aren't
breaking the plugin. Unit-tests can only do so much with the amount of
inter-tool interaction we have


Reply to this email directly or view it on GitHub
#470 (comment)
.

@advayDev1
Copy link
Contributor

Is 'it' our plugin? Or guava as the system-under-test?

Either way I think that's the whole benefit of Travis. We don't need to serially spend time (or our local CPU cycles) waiting those 18 minutes. You push and do other work in parallel. Like with any presubmit, devs usually have enough pipelined work that a remote system thoroughly testing shouldn't delay work.

Also Travis has a feature such that for example if you are strictly changing only CONTRIBUTING or README, etc. add '[skip ci] anywhere in the commit, and Travis will auto-green the CL.

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.

2 participants