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

Update constants.rb #182

Closed
wants to merge 1 commit into from
Closed

Conversation

chintan1891
Copy link

Resolved Blank Other linker flag issue

Resolved Other linker flag issue
@alloy
Copy link
Member

alloy commented Sep 5, 2014

Please provide at least a rational for your change.

@chintan1891
Copy link
Author

I am facing issue where my CocoaPods XCConfing are overridden by blank OTHER_LDFLAGS : liftoffcli/liftoff#111

@alloy
Copy link
Member

alloy commented Sep 5, 2014

I see. What I don’t understand is that it was already removed earlier in #140.

@fabiopelosin Do you have any insight as to what’s going on here?

@gfontenot
Copy link
Contributor

Looks like my change was reverted in 313c732. I remember it being reverted, don't recall why.

@fabiopelosin
Copy link
Member

The @gfontenot patch was reverted in a hurry in the sister release of CocoaPods I received a couple of reports which lead me to believe that a non negligible number of CocoaPods installations relied on this behaviour and I didn't want to break them.

Unfortunately i don't recall the exact details... however Xcodeproj should just limit itself to mimic Xcode behaviour (which if I am correct is to not include this setting). If CocoaPod for any reason relies on it, it is its responsibility to set it and document why it is needed.

@alloy
Copy link
Member

alloy commented Sep 12, 2014

Sounds good 👍

@fabiopelosin
Copy link
Member

As Xcodeproj has already been released this can be merged as soon as ready. @chintan1891 can you add a note to the Changelog for the change crediting yourself and @gfontenot?

@kylef
Copy link
Contributor

kylef commented Nov 7, 2014

Thanks for your contribution @chintan1891, I'm going to close this because it's covered in #166.

@kylef kylef closed this Nov 7, 2014
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.

5 participants