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

README updates #315

Merged
merged 1 commit into from
Jan 24, 2021
Merged

README updates #315

merged 1 commit into from
Jan 24, 2021

Conversation

pogramos
Copy link

This is a doc update with the usage hints, based on the problems found in Xcode 11 and as stated in this Pull Request #314

@orta
Copy link
Collaborator

orta commented Oct 24, 2019

Cool!

Though, that PR renames constrain to cg_constrain - so the README would be different if I merged that. Which do you prefer?

@pogramos
Copy link
Author

using cg_constrain would do no harm, but it would be good if the old functions were deprecated instead of directly renaming it... considering that there could be some projects using the framework without a version lock, it could cause lots of crashes, so IMO we could use something like:

@available(swift, deprecated: 5.1, renamed: "cg_constrain") 

on the old functions to point out that it will not be supported in the near future and users should migrate to this naming convention.
So later on those functions could be safely removed.

What do you think?

@pogramos
Copy link
Author

pogramos commented Nov 6, 2019

I've put some thought on it, and I think we should not rename constrain to cg_constrain, since we have Swift's modularity to play on our side;
Using Cartography.constrain just like a namespace would be a benefit in some ways since we'll always know where that constrain function came from.

IMO we could just leave it as is, and just push the docs update.

@orta orta merged commit f34f613 into robb:master Jan 24, 2021
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