Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 changelog/contributors/version bump in one script #19379
Update changelog/contributors/version bump in one script #19379
Changes from 10 commits
c0b6564
db7fd89
f710e53
469f0e7
bcc624c
1ec6054
9704893
513c079
d377366
2070422
248e7d5
714b627
c18b4ed
b4d8250
9fa9c90
66d4269
d5a9546
2e863fb
00786d3
0c60373
5fd9e8c
554a15b
62ae8b1
36a60cf
29520a4
f0e56b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
do we want to extract this into a method in git.py to keep all the git in one place?
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.
For one-off/specialised calls like this, I don't have a particular point of view. For me, I'd lean away from wrapping this sort of thing up into a "
find_two_most_recent_commits_changing_version
" function unless there were more locations that needed it! 🤷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.
That's fair. There's always a tension between keeping everything at the same abstraction level and having a pile of hyper-specific functions. I find that naming the function by why I would use it helps me determine if it's worth wrapping. (Put another way: the view of this interface by the consumer rather than the provider.) something like
get_commit_filter_for_this_release
would fit more naturally in a list of steps that this function performs, like:You're right that this would make our git.py contain a lot of one-off functions.