Skip to content
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

[ENH]: Filter PRs by checking for commits in branch #54

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

manics
Copy link
Contributor

@manics manics commented May 14, 2021

If a branch is specified PRs are filtered by checking their PR base branch. This fails to pick up all PRs when multiple PRs are merged into one branch, followed by that branch being merged into the mainline via a single PR. It will also fail if a branch is renamed.

If the --since argument is a git ref (not a date) this will now be used to get a list of commits in the requested branch after --since. The list of PRs is then filtered by checking whether the PR commit appears in this list of branch commits. If the commit history for merged PRs/branches is maintained this should ensure PRs merged into one branch that are subsequently merged altogether into another branch are picked up.

If --since is a date the current behaviour is kept since a commit date is not the same as the date the commit was added to a repo, so the only alternative is to fetch the list of all commits.

Closes #50

Todo:

  • Tests

If a branch is specified PRs are filtered by checking their PR base branch.
This fails to pick up all PRs when multiple PRs are merged into one branch, followed by that branch being merged into the mainline via a single PR.

If the `--since` argument is a git ref (_not_ a date) this is used to get a list of commits in the requested branch after `--since`. The list of PRs is then filtered by checking whether the PR commit appears in this list of branch commits. If the commit history for merged PRs/branches is maintained this should ensure PRs merged into one branch that are subsequently merged altogether into another branch are picked up.

Closes executablebooks#50
@manics
Copy link
Contributor Author

manics commented May 14, 2021

Example: JupyterHub was switched from master to main after 1.4.0:


Old behaviour: github-activity --target jupyterhub/jupyterhub --branch main --since 1.4.0

# 1.4.0...master@{2021-05-14}

([full changelog](https://github.com/jupyterhub/jupyterhub/compare/1.4.0...6be3160d7487bb362df7095f24454ef2814951fc))

## Merged PRs

- prepare to rename default branch to main [#3462](https://github.com/jupyterhub/jupyterhub/pull/3462) ([@minrk](https://github.com/minrk))

## Contributors to this release
...

Old behaviour: github-activity --target jupyterhub/jupyterhub --branch master --since 1.4.0

# 1.4.0...master@{2021-05-14}

([full changelog](https://github.com/jupyterhub/jupyterhub/compare/1.4.0...77691ae4023e68ae0ef1a21ca5301bebb5107bc3))

## New features added

- Support Pagination in the REST API [#3413](https://github.com/jupyterhub/jupyterhub/pull/3413) ([@naatebarber](https://github.com/naatebarber))

## Enhancements made

- patch base handlers from both jupyter_server and notebook [#3437](https://github.com/jupyterhub/jupyterhub/pull/3437) ([@minrk](https://github.com/minrk))

## Bugs fixed

- define Spawner.delete_forever on base Spawner [#3454](https://github.com/jupyterhub/jupyterhub/pull/3454) ([@minrk](https://github.com/minrk))

## Maintenance and upkeep improvements

- ci: fix typo in environment variable [#3457](https://github.com/jupyterhub/jupyterhub/pull/3457) ([@consideRatio](https://github.com/consideRatio))
- avoid re-using asyncio.Locks across event loops [#3456](https://github.com/jupyterhub/jupyterhub/pull/3456) ([@minrk](https://github.com/minrk))
- ci: github workflow security, pin action to sha etc [#3436](https://github.com/jupyterhub/jupyterhub/pull/3436) ([@consideRatio](https://github.com/consideRatio))
- Remove handling of jupyterhub 0.8 oauth client ids [#3433](https://github.com/jupyterhub/jupyterhub/pull/3433) ([@minrk](https://github.com/minrk))

## Documentation improvements

- Fix documentation [#3452](https://github.com/jupyterhub/jupyterhub/pull/3452) ([@davidbrochart](https://github.com/davidbrochart))

## Contributors to this release
...

New behaviour: github-activity --target jupyterhub/jupyterhub --branch main --since 1.4.0

# 1.4.0...master@{2021-05-14}

([full changelog](https://github.com/jupyterhub/jupyterhub/compare/1.4.0...6be3160d7487bb362df7095f24454ef2814951fc))

## New features added

- Support Pagination in the REST API [#3413](https://github.com/jupyterhub/jupyterhub/pull/3413) ([@naatebarber](https://github.com/naatebarber))

## Enhancements made

- patch base handlers from both jupyter_server and notebook [#3437](https://github.com/jupyterhub/jupyterhub/pull/3437) ([@minrk](https://github.com/minrk))

## Bugs fixed

- define Spawner.delete_forever on base Spawner [#3454](https://github.com/jupyterhub/jupyterhub/pull/3454) ([@minrk](https://github.com/minrk))

## Maintenance and upkeep improvements

- ci: fix typo in environment variable [#3457](https://github.com/jupyterhub/jupyterhub/pull/3457) ([@consideRatio](https://github.com/consideRatio))
- avoid re-using asyncio.Locks across event loops [#3456](https://github.com/jupyterhub/jupyterhub/pull/3456) ([@minrk](https://github.com/minrk))
- ci: github workflow security, pin action to sha etc [#3436](https://github.com/jupyterhub/jupyterhub/pull/3436) ([@consideRatio](https://github.com/consideRatio))
- Remove handling of jupyterhub 0.8 oauth client ids [#3433](https://github.com/jupyterhub/jupyterhub/pull/3433) ([@minrk](https://github.com/minrk))

## Documentation improvements

- Fix documentation [#3452](https://github.com/jupyterhub/jupyterhub/pull/3452) ([@davidbrochart](https://github.com/davidbrochart))

## Other merged PRs

- prepare to rename default branch to main [#3462](https://github.com/jupyterhub/jupyterhub/pull/3462) ([@minrk](https://github.com/minrk))

## Contributors to this release
...

@choldgraf
Copy link
Member

This seems pretty good to me! If @consideRatio is happy with the implementation I am 👍

@consideRatio
Copy link
Collaborator

consideRatio commented May 16, 2021

I note that this is a Draft PR currently. I'll wait with code review until its a non-draft. @manics please ping me if its ready for review now or in the future!

@manics
Copy link
Contributor Author

manics commented Jul 24, 2021

I'm not sure how to add a test for this... maybe a dedicated branch we can run tests against for real?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exclude PRs merged but not in releasing branch
3 participants