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

Upgrade QuoteFix to 2.7.3 #16157

Closed
wants to merge 1 commit into from
Closed

Conversation

zmwangx
Copy link
Contributor

@zmwangx zmwangx commented Dec 27, 2015

Also changed license from :oss to :mit. (Check https://github.com/robertklep/quotefixformac#licence--copyright. It doesn't explicitly say MIT, but the license terms are clearly MIT.)

Oops I mistook the included license of Sparkle for the license of QuoteFix. Reverted the license change.

@adidalal
Copy link
Contributor

Appcast shasum should also be updated, in future PRs.
This is fine for the time being, as we don't yet have anything in place to auto-update, but Soon(TM)

@adidalal
Copy link
Contributor

Thank you for the contribution. It was merged directly as acbfde7 to keep commit history cleaner. Your contribution is still credited to you.

@adidalal adidalal closed this Dec 27, 2015
@zmwangx
Copy link
Contributor Author

zmwangx commented Dec 27, 2015

Didn't even realize appcast also has a checksum now... Thanks for the heads up.

@zmwangx zmwangx deleted the quotefix-2.7.3 branch December 27, 2015 21:06
@adidalal
Copy link
Contributor

https://github.com/vitorgalvao/tiny-scripts/blob/master/cask-repair is also useful for automating the process, fyi.

@zmwangx
Copy link
Contributor Author

zmwangx commented Dec 27, 2015

Thanks for the info. Personally I won't use it because the current version (vitorgalvao/tiny-scripts@deb849d) clears the Homebrew download cache, but I can see it's a nice and useful script, and maybe I'll fork it someday if manual editing becomes too annoying.

@vitorgalvao
Copy link
Member

the current version (…) clears the Homebrew download cache

Not anymore. I wasn’t doing it for any particular reason other than convenience (to me, when coding it). If that makes a difference to someone, though, no quarrel with changing it. Code isn’t exactly uglier and the end result is the same without removing something you didn’t ask to be removed.

Largely untested, but should work.

@zmwangx
Copy link
Contributor Author

zmwangx commented Dec 27, 2015

Cool, thanks for the quick change. Nitpick: You need to use /usr/bin/stat to prevent picking up coreutils stat which may be default for some (e.g. me) but is incompatible with BSD stat.

Due to the largely untested nature, I'll need to fully review it before daring to run it on my system, and I don't have time for that now. But the script could potentially be a time saver for regular cask contributors, so maybe a pointer in an appropriate doc page would be nice (or maybe there already is one)?

@vitorgalvao
Copy link
Member

use /usr/bin/stat to prevent picking up coreutils stat which may be default for some (e.g. me) but is incompatible with BSD stat.

Good point. Will change it.

Due to the largely untested nature

To be clear, the script has been throughly tested and is used by many people without any (reported) issue. The only thing I haven’t tested is this small change, but it’s not like it’ll break anything. Worse case scenario, it could get the wrong file and you’d submit a wrong shasum (that would be caught by Travis) or it wouldn’t work at all.

To reiterate, there isn’t anything anything in this change that’ll break your system, and every other part of the script has been tested and is in use.

But the script could potentially be a time saver for regular cask contributors, so maybe a pointer in an appropriate doc page would be nice (or maybe there already is one)?

maintaining.md.

@zmwangx
Copy link
Contributor Author

zmwangx commented Dec 27, 2015

To be clear, the script has been throughly tested and is used by many people without any (reported) issue.

Okay, misinterpreted. Sorry.

@zmwangx zmwangx mentioned this pull request Jan 7, 2016
8 tasks
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants