-
-
Notifications
You must be signed in to change notification settings - Fork 648
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
Changes from all 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
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,18 +2,17 @@ | |
# Licensed under the Apache License, Version 2.0 (see LICENSE). | ||
import argparse | ||
import json | ||
from pathlib import Path | ||
|
||
from pants_release.common import sorted_contributors | ||
from pants_release.common import VERSION_PATH, sorted_contributors | ||
from pants_release.git import git | ||
|
||
from pants.util.strutil import softwrap | ||
|
||
|
||
def announcement_text() -> str: | ||
version = Path("src/python/pants/VERSION").read_text().strip() | ||
version = VERSION_PATH.read_text().strip() | ||
cur_version_sha, prev_version_sha = git( | ||
"log", "-2", "--pretty=format:%h", "src/python/pants/VERSION" | ||
"log", "-2", "--pretty=format:%h", str(VERSION_PATH) | ||
).splitlines(keepends=False) | ||
Comment on lines
14
to
16
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 " There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
You're right that this would make our git.py contain a lot of one-off functions. |
||
git_range = f"{prev_version_sha}..{cur_version_sha}" | ||
all_contributors = sorted_contributors(git_range) | ||
|
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 can enforce this, by adding
("[PyGithub]", "src/python/pants_release/**", "!*"),
topants/3rdparty/python/BUILD
Lines 36 to 49 in 00786d3
Something like:
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.
As a slight aside, should we swap so the core pants package is includelisted, and everything else is free game?
Because it shouldn't matter if a release needs
fastapi
(I mean why, but you get the point) or some non-included plugin usesPyGitHub
(what if its a GitHub plugin?)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.
I've filed #19458
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 works.