-
Notifications
You must be signed in to change notification settings - Fork 49
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
Implement generation status #12
Conversation
This patch utilizes a file to track XCHammer updates. When the generator successfully runs, we create the file.
tempProjectPath) | ||
guard FileManager.default.createFile(atPath: genStatusPath, | ||
contents: "".data(using: .utf8), attributes: nil) else { | ||
fatalError("Can't write genStatus") |
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.
What does this mean for people that run xchammer
? We should have something helpful here
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.
None of the of the fatals here are meaningful to users, as they're invalid program states ( fatal errors ). Perhaps we should cull them into a fatal XCHammer
that is more clear.
xcworkspacePath) | ||
guard FileManager.default.createFile(atPath: genStatusPath, | ||
contents: "".data(using: .utf8), attributes: nil) else { | ||
fatalError("Can't write genStatus") |
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.
Same here
if [[ $ACTION == "clean" ]]; then | ||
exit 0 | ||
fi | ||
|
||
PREV_STAT=`stat -f %c "$0"` | ||
PREV_STAT=`stat -f %c "/Users/jerry/Projects/xchammer-github/sample/UrlGet/UrlGet.xcodeproj/XCHammerAssets/genStatus"` |
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.
are you expecting absolute paths here? remove
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.
Yep we're using absolute paths, and unfortunately, they are all hardcoded to the last persons computer.
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.
We should have a better way..
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 the review @rahul-malik - I filed a ticket to rework the sample to a golden file |
This patch utilizes a file to track XCHammer updates.
When the generator successfully runs, we create the file.