Skip to content

Comments

Remove special casing for objc.d from the Posix makefile#1636

Merged
schuetzm merged 1 commit intodlang:masterfrom
jacob-carlborg:objc_refactor
Apr 25, 2017
Merged

Remove special casing for objc.d from the Posix makefile#1636
schuetzm merged 1 commit intodlang:masterfrom
jacob-carlborg:objc_refactor

Conversation

@jacob-carlborg
Copy link
Contributor

This file is now shared for all platforms after the Objective-C integration refactoring: dlang/dmd#6679.

This file is now shared for all platforms after the Objective-C
integration refactoring: dlang/dmd#6679.
@jacob-carlborg
Copy link
Contributor Author

@ibuclaw @WalterBright This is required for the DAutoTest to pass for dlang/dmd#6679.

I guess this will fail until dlang/dmd#6679 is merged.

@ibuclaw
Copy link
Member

ibuclaw commented Apr 17, 2017

Looks fine. FYI @CyberShadow.

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

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

I guess the same will need to be done for objc_glue too if you plan on changing that too?

@jacob-carlborg
Copy link
Contributor Author

I guess the same will need to be done for objc_glue too if you plan on changing that too?

Yes, one step at the time 😃. I'll wait until I have the PR for the compiler ready.

@CyberShadow
Copy link
Member

LGTM, but I'm not going to merge this because it looks like by itself it would break master. I understand that merging this simultaneously with dlang/dmd#6679 would fix things again, so whoever merges dlang/dmd#6679 should merge this immediately after.

Even better would be to do this in three steps to avoid breaking master at all. E.g. in the dlang.org makefile, detect whether objc.d should be ignored (i.e. whether dlang/dmd#6679 is merged). That would completely avoid the risk of breaking master should merging both PRs somehow still result in tests failing.

@jacob-carlborg
Copy link
Contributor Author

@CyberShadow auto merging has been enabled for the other PR dlang/dmd#6679, can this be merged now? Looks like Iain doesn't have permission to merge this.

@jacob-carlborg
Copy link
Contributor Author

@CyberShadow dlang/dmd#6679 has been merged. This can now be merged as well.

@schuetzm schuetzm merged commit d581bd3 into dlang:master Apr 25, 2017
@jacob-carlborg
Copy link
Contributor Author

Thanks.

@jacob-carlborg jacob-carlborg deleted the objc_refactor branch April 25, 2017 11:21
@CyberShadow
Copy link
Member

Thanks for handling this @schuetzm, just got back home :)

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.

4 participants