-
Notifications
You must be signed in to change notification settings - Fork 311
Release notes generator script #4866
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
base: master
Are you sure you want to change the base?
Conversation
… filter out bugs behind features that are not yet shipped
bugbug/tools/release_notes.py
Outdated
return "\n".join(unique_lines) | ||
|
||
def get_final_release_notes_commits( | ||
self, target_version: 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.
Would taking the release and the channel separately, instead of one string, be cleaner?
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 think the way we're getting the commit logs from the URL, it should be fine. However, if you think it would be better, I can change it to have two inputs rather than a single string for the release and channel.
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 think it would be clear from an API point of view.
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.
Done here: 38499c3
bugbug/tools/release_notes.py
Outdated
|
||
bug_ids = [] | ||
for desc, _, _ in commit_log_list: | ||
match = re.search(r"Bug (\d+)", desc, re.IGNORECASE) |
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 parse the commit message twice, once here and again in filter_irrelevant_commits()
. It would be nice to do it once here; we could have a list of structured data about each commit, e.g., a tuple or dict with the bug number and message.
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.
Done here! aebee0a
bugbug/tools/release_notes.py
Outdated
def get_previous_version(target_version: str) -> str: | ||
match = re.search(r"(\d+)", target_version) | ||
if not match: | ||
raise ValueError("No number found in the version string") | ||
|
||
number = match.group(0) | ||
decremented_number = str(int(number) - 1) | ||
return ( | ||
target_version[: match.start()] | ||
+ decremented_number | ||
+ target_version[match.end() :] | ||
) |
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.
Is this still needed?
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.
No, thank you for pointing this out. I've removed it!
tests/test_release_notes.py
Outdated
def test_get_previous_version(): | ||
assert get_previous_version("FIREFOX_BETA_135_BASE") == "FIREFOX_BETA_134_BASE" | ||
assert get_previous_version("FIREFOX_NIGHTLY_132") == "FIREFOX_NIGHTLY_131" | ||
assert get_previous_version("FIREFOX_RELEASE_130_2") == "FIREFOX_RELEASE_129_2" |
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.
How do we now handle cases like FIREFOX_RELEASE_130_2
?
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 think I just had that case there to make sure the string parsing was correct. Now that we specify the channel and release separately, would there be a case of FIREFOX_RELEASE_130_2
or FIREFOX_RELEASE_130_2_BASE
? I've tried to retrieve the commits from the URL using these but was unable to (maybe because these do not exist?).
Includes both the tool (
bugbug/tools/release_notes.py
) and the runner (scripts/release_notes_runner.py
).Takes two parameters:
--version
- the version of Firefox to generate a list of potential commits to include in release notes (e.g.FIREFOX_BETA_136_BASE
)--chunk-size
- the size of the chunk (in tokens) when passing in the commit logs to the LLM