-
-
Notifications
You must be signed in to change notification settings - Fork 638
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
A script to generate a release announcement message. #19361
Conversation
Currently renders as:
|
I took the liberty of changing the phrasing of our current announcement. For example, we currently link to PyPI from force of habit, but we actually don't want to draw attention to the fact that we happen to currently release that way. It might put people in mind of being able to pip-install Pants, which we definitely don't want them to do. Plugin developers will have read enough to know that Pants is available on PyPI, but there is no need to draw attention to that for the general public. |
A future PR will wire this up to email and Slack.
631e10c
to
fd004cb
Compare
else: | ||
update_contributors_md() | ||
|
||
|
||
def sorted_contributors(range: str) -> list[str]: |
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.
Moved into common.py
@@ -31,3 +32,17 @@ def elapsed_time() -> tuple[int, int]: | |||
now = time.time() | |||
elapsed_seconds = int(now - _SCRIPT_START_TIME) | |||
return elapsed_seconds // 60, elapsed_seconds % 60 | |||
|
|||
|
|||
def sorted_contributors(git_range: str) -> list[str]: |
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.
Moved from contributors.py and arg renamed from range to git_range, since range is a Python builtin
@@ -49,6 +49,7 @@ plugins = [ | |||
pantsd_invalidation_globs.add = [ | |||
"!*_test.py", | |||
"!BUILD", | |||
"!src/python/pants_release/**", |
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.
Ah, thanks, having to wait for pantsd to restart for any code change there has been frustrating. I didn't realise it was resolvable!
] | ||
) | ||
|
||
announcement = softwrap( |
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.
Would it make sense to start using Jinja for string templates? I find string templates much more readable and easier to debug.
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 don't think so for this change. It's too trivial. And we're trying to reduce deps in the repo, right?
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 a more substantial thing, possibly? But then we'd have to take steps to ensure it doesn't end up used in the pants binary itself? So not sure it's worth the complication.
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 could do that with separate resolves I suppose! But anyway, for a followup at best. And I think I'd like to see a more acute need?
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 and should use 3rdparty for our helper scripts. Again, I love 3rdparty 😛 I just think build tools are themselves are special 😄
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 also don't think this so so trivial it wouldn't need a template. all the strings and newlines and branching and loops makes my brain just turn off 😂
But that could just be me.
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 and should use 3rdparty for our helper scripts. Again, I love 3rdparty 😛 I just think build tools are themselves are special 😄
But then how do you prevent people from using it in Pants itself? Now we need more guardrails.
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 like avoiding any 3rd party unless justified. Fwiw on guards, we could use the visibility rules introduced to ensure anything under SRC/pants only has access to a specific set of dependencies
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 we're currently using a sophisticated build tool with guardrails built in 😉
But also, we have "guardrails" today, they're just made of paper-mache
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.
LGMT!
A future PR will wire this up to email and Slack.