-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 .gitignore file [Carthage] #1581
Conversation
.gitignore
Outdated
@@ -32,3 +32,11 @@ playground.xcworkspace | |||
/.buckconfig.local | |||
/.buckd | |||
|
|||
# CocoaPods | |||
*.xcworkspace | |||
Podfile.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://guides.cocoapods.org/using/using-cocoapods.html -> "Whether or not you check in the Pods directory, the Podfile and Podfile.lock should always be kept under version control."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @wiseoldduck , thanks for pointing out this, the commit is updated :)
btw, it seems the repo's example projects don't host Podfile.lock
at the moment, how about adding them to be under version control?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah examples ideally should have Podfile.lock
. Not a big deal I'd say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticing the build.sh
, which will be ran by CI, ignores Podfile.lock
in the example projects every time it makes a new build
...
if [ -f "${example}/Podfile" ]; then
echo "Using CocoaPods"
if [ -f "${example}/Podfile.lock" ]; then
rm "$example/Podfile.lock"
fi
...
So I think it's safe to not commit those Podfile.lock
at the moment :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting up the PR!
.gitignore
Outdated
@@ -32,3 +32,10 @@ playground.xcworkspace | |||
/.buckconfig.local | |||
/.buckd | |||
|
|||
# CocoaPods | |||
*.xcworkspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not ignore AsyncDisplayKit.xcworkspace
? We've been using it to sync some project-wide settings across the team (see its commit history for more details).
Also just realized that this line in the dev doc is not correct. We should fix it as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for not paying enough attention to it. Let's keep .xcworkspace
files for this purpose. I've updated the commit to just ignore Carthage
related files this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify why do we need to fix this line?
Run `pod install` in the directory that you cloned to.
Because the Pods
folder is not kept under version control, it seems we need to run pod install
after cloning the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, you're right. That line is fine.
Quoting from Carthage's README.md on Github: "Along the way, Carthage will have created some build artifacts. The most important of these is the Cartfile.resolved file, which lists the versions that were actually built for each framework. Make sure to commit your Cartfile.resolved, because anyone else using the project will need that file to build the same framework versions."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Will merge when CI passes.
🚫 CI failed with log |
Update .gitignore file [Carthage]