Improve branch filtering and raise error on empty dataframes #162
Conversation
| ## Filter pull requests by branch | ||
|
|
||
| Many repositories work with multiple active branches (e.g., `main`, `develop`, feature branches). When generating a changelog for a specific release, you typically only want to include pull requests that were merged into the release branch. | ||
|
|
||
| Use the `--branch` (or `-b`) parameter to filter pull requests by their target branch: | ||
|
|
||
| ```bash | ||
| github-activity org/repo --since v1.0.0 --until v2.0.0 --branch main | ||
| ``` | ||
|
|
||
| This will **only include pull requests that targeted the `main` branch**, excluding any PRs merged to other branches like `develop` or feature branches. | ||
|
|
||
| ```{note} | ||
| You can use any git reference (tag, commit hash, etc.) in place of a branch name. |
There was a problem hiding this comment.
This is just documenting pre-existing functionality
| # Wrap in a try/except so we don't have an ugly stack trace if there's an error | ||
| try: | ||
| if args.all: | ||
| md = generate_all_activity_md(args.target, **common_kwargs) |
There was a problem hiding this comment.
This is all just tab-adding. The only difference is the try/except statement, so that if valueerrors happen down below we can display them nicely.
|
I gave this a quick review just to sanity check - it seems OK to me and it makes our tests happy, so I'll merge this and cut a quick release since this fixes our tests! |
| target, since=since, until=until, kind=kind, auth=auth, cache=False | ||
| ) | ||
|
|
||
| # Raise error if GitHub API returned no activity at all |
There was a problem hiding this comment.
Noting this could be causing a change of behavior in the jupyter releaser, as noticed in some workflows today:
https://github.com/jtpio/jupyterlite-server-contents/actions/runs/20604073817/job/59175936047
ValueError: No activity found for jtpio/jupyterlite-server-contents between v0.1.1 and None.
There was a problem hiding this comment.
Since the releaser was likely not expecting this call to raise, wondering if this could be considered somewhat breaking?
There was a problem hiding this comment.
Ah - maybe, I didn't realize it was something that people were hitting. Maybe we need to think through the intended behavior better, can you open an issue for what you expect to happen vs what happens here?
There was a problem hiding this comment.
Probably it's ok if it throws now, jupyter-server/jupyter_releaser#637 should handle that now in the releaser.
Maybe what could be surprising could be that change of behavior in a patch release. Although that wouldn't change the fact that the releaser would need to handle that case since it does not pin on minor releases of github-activity.
There was a problem hiding this comment.
Yeah I hadn't considered that people would be hitting that bug, I thought it was an edge case but maybe I didn't think through the logic properly. Are you suggesting that we yank the last release and make it a minor release?
I guess it's an interesting philosophical question - what do you do if a bug has been around long enough that users have built workflows that expect the bug, so fixing the bug will actually break workflows
There was a problem hiding this comment.
Thinking more about this: isn't the case of no activity a valid case?
Should this really raise if there is no data?
github-activity/github_activity/github_activity.py
Lines 466 to 471 in 82fc6bd
2 participants
This fixes two things I noticed in #161
None)maindue to queries that return an empty result #161