-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
dev-cmd/unbottled: add --lost option #16115
dev-cmd/unbottled: add --lost option #16115
Conversation
This option tries to find bottles where the bottle was removed from the formula in the past week and not added back in. It checks the output of `git log` to see if there are any bottles that fit this criteria.
534490c
to
20f00a6
Compare
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.
Looks good to me so far! I'd be fine with merging once @Bo98 and/or @fxcoudert are 👍🏻 and you've considered my questions. Thanks again @apainintheneck!
git_log << "-G^ +sha256.* #{@bottle_tag}:" | ||
git_log << "--since=@{'1 week ago'}" | ||
|
||
bottle_tag_sha_regex = /^[+-] +sha256.* #{@bottle_tag}: / |
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.
Might be good to not have a copy-paste of common part of the regex with the earlier regex.
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.
Not sure what you mean by this. Are you suggesting adding another variable to hold shared parts of both regexes?
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
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.
Sounds good, handled in e48c155
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.
Optional, but perhaps slightly safer code in case it picks up anything in documentation or whatever
Seems very nice to me! |
I like this idea too. |
- Add heading message to indicate bottle tag used - Use --no-ext-diff with git to avoid potential problems - Resolve formula renames to avoid false positives - Increase parsing stringency
This makes it easier to see that the two regexes are related and could make it easier to change them in the future if need be.
Thanks again @apainintheneck! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Closes #16016
This option tries to find bottles where the bottle was removed from the formula in the past week and not added back in.
It checks the output of
git log
to see if there are any bottles that fit this criteria.Note: This implementation does not handle renamed formula well but I assume that doesn't happen that often so having the occasional false positive is not a big deal here.
The
git log
output looks something like this for each commit.We parse the commit, formula name and bottle changes from this and then print that the bottle is lost if the bottle was removed and not replaced.
Examples: