Skip to content

Commit

Permalink
feat(main): support pr review collection options (#40)
Browse files Browse the repository at this point in the history
* chore(deps): add cloup

* feat(collection): support pr review options

* feat(main): add pr review collection options

* style(main): remove whitespace

* docs(readme): add user reviewed and user requested review sections

* docs(readme): updated learn more about date filters note

* docs(readme): remove space
  • Loading branch information
kiran94 authored May 27, 2023
1 parent 17566e9 commit b451bd6
Show file tree
Hide file tree
Showing 7 changed files with 150 additions and 30 deletions.
26 changes: 24 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ prfiesta -u kiran94 -dc events_url -dc comments_url -dc node_id
# Get all pull requests where the user was involved (as opposed to just authored)
prfiesta -u kiran94 --use-involves

# Get all pull requests where the user reviewed it rather then being the author
prfiesta -u charliermarsh --after 2023-05-01 --use-reviewed-by

# Get all pull requests where the user was requested a review rather then being the author
prfiesta -u charliermarsh --after 2023-05-01 --use-review-requested

# Get help
prfiesta --help

Expand Down Expand Up @@ -85,7 +91,11 @@ You can also customize the output file name using the `--output` option.

By default, `prfiesta` will take the users provided in the `--user` option and search the Git provider for any pull requests that the user **authored**. Within more collaborative environments, this may not be what you want as you may want to also gain some visibility into all secondary contributions a user made (e.g commenting on others pull requests).

To help with this, `prfiesta` exposes the `--use-involves` flag which will search for pull requests that were:
*The options listed here are mutually exclusive.*

#### User Involvement

`prfiesta` exposes the `--use-involves` flag which will search for pull requests that were:

- Created by a certain user
- Assigned to that user
Expand All @@ -94,11 +104,23 @@ To help with this, `prfiesta` exposes the `--use-involves` flag which will searc

Learn more about `involves` [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-a-user-thats-involved-in-an-issue-or-pull-request).

#### User Reviewed

`prfiesta` exposes a `--use-reviewed-by` flag which will collect pull requests where the user has reviewed others pull requests.

Learn more about searching review requests [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-pull-request-review-status-and-reviewer)

#### User Requested Review

`prfiesta` exposes a `--use-review-requested` flag which will collect pull requests where the user was *requested* a review from other collaborators.

Learn more about searching review requests [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-pull-request-review-status-and-reviewer)

### Date Filter

When using the `--after` and `--before` date filters, by default `prfiesta` will use the `created` date dimension with these filters on the Git provider (e.g GitHub). This may not fit your use case and you may want to filter on when a pull request was updated instead. To do this you can use the `--use-updated` flag.

See more information [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-when-an-issue-or-pull-request-was-created-or-last-updated).
Learn more about date filters [here](https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests#search-by-when-an-issue-or-pull-request-was-created-or-last-updated).

## Analysis

Expand Down
18 changes: 17 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 30 additions & 14 deletions prfiesta/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import datetime

import click
import cloup
from rich.live import Live
from rich.spinner import Spinner
from rich.text import Text
Expand All @@ -15,19 +16,25 @@

github_environment = GitHubEnvironment()


@click.command()
@click.option('-u', '--users', required=True, multiple=True, help='The GitHub Users to search for. Can be multiple')
@click.option('-a', '--after', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests after this date e.g 2023-01-01')
@click.option('-b', '--before', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests before this date e.g 2023-04-30')
@click.option('-d', '--use-updated', is_flag=True, default=False, help='filter on when the pr was last updated rather then created')
@click.option('-i', '--use-involves', is_flag=True, default=False, help='include prs where the author was the author or assignee or mentioned or commented')
@click.option('-dc', '--drop-columns', multiple=True, help='Drop columns from the output dataframe')
@click.option('-x', '--url', help='The URL of the Git provider to use')
@click.option('-t', '--token', help='The Authentication token to use')
@click.option('-o', '--output', default=None, help='The output location')
@click.option('-ot', '--output-type', type=click.Choice(['csv', 'parquet']), default='csv', show_default=True, show_choices=True, help='The output format')
@click.version_option(__version__)
@cloup.command()
@cloup.option('-u', '--users', required=True, multiple=True, help='The GitHub Users to search for. Can be multiple')
@cloup.option('-a', '--after', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests after this date e.g 2023-01-01')
@cloup.option('-b', '--before', type=click.DateTime(formats=['%Y-%m-%d']), help='Only search for pull requests before this date e.g 2023-04-30')
@cloup.option('-d', '--use-updated', is_flag=True, default=False, help='filter on when the pr was last updated rather then created')
@cloup.option_group(
'Author Filter',
cloup.option('-i', '--use-involves', is_flag=True, default=False, help='collect prs where the users are the author or assignee or mentioned or commented'),
cloup.option('-r', '--use-reviewed-by', is_flag=True, default=False, help='collect prs where the users reviewed them'),
cloup.option('-rr', '--use-review-requested', is_flag=True, default=False, help='collect prs where the users were requested a review'),
constraint=cloup.constraints.mutually_exclusive,
help='Collect alternative details for the users. If omitted then just collect the prs that a user has authored.',
)
@cloup.option('-dc', '--drop-columns', multiple=True, help='Drop columns from the output dataframe')
@cloup.option('-x', '--url', help='The URL of the Git provider to use')
@cloup.option('-t', '--token', help='The Authentication token to use')
@cloup.option('-o', '--output', default=None, help='The output location')
@cloup.option('-ot', '--output-type', type=click.Choice(['csv', 'parquet']), default='csv', show_default=True, show_choices=True, help='The output format')
@cloup.version_option(__version__)
def main(**kwargs) -> None:

users: tuple[str] = kwargs.get('users')
Expand All @@ -40,6 +47,8 @@ def main(**kwargs) -> None:
drop_columns: list[str] = list(kwargs.get('drop_columns'))
use_updated: bool = kwargs.get('use_updated')
use_involves: bool = kwargs.get('use_involves')
use_reviewed_by: bool = kwargs.get('use_reviewed_by')
use_review_requested: bool = kwargs.get('use_review_requested')

logger.info('[bold green]PR Fiesta 🦜🥳')

Expand All @@ -48,7 +57,14 @@ def main(**kwargs) -> None:
with Live(spinner, refresh_per_second=20, transient=True):

collector = GitHubCollector(token=token, url=url, spinner=spinner, drop_columns=drop_columns)
pr_frame = collector.collect(*users, after=after, before=before, use_updated=use_updated, use_involves=use_involves)
pr_frame = collector.collect(
*users,
after=after,
before=before,
use_updated=use_updated,
use_involves=use_involves,
use_reviewed_by=use_reviewed_by,
use_review_requested=use_review_requested)

if not pr_frame.empty:
logger.info('Found [bold green]%s[/bold green] pull requests!', pr_frame.shape[0])
Expand Down
22 changes: 17 additions & 5 deletions prfiesta/collectors/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ def collect(
before: Optional[datetime] = None,
use_updated: Optional[bool] = False,
use_involves: Optional[bool] = False,
use_reviewed_by: Optional[bool] = False,
use_review_requested: Optional[bool] = False,
) -> pd.DataFrame:

query = self._construct_query(users, after, before, use_updated, use_involves)
query = self._construct_query(users, after, before, use_updated, use_involves, use_reviewed_by, use_review_requested)

update_spinner(f'Searching {self._url} with[bold blue] {query}', self._spinner, logger)

Expand Down Expand Up @@ -92,6 +94,8 @@ def _construct_query(
before: Optional[datetime] = None,
use_updated: Optional[bool] = False,
use_involves: Optional[bool] = False,
use_reviewed_by: Optional[bool] = False,
use_review_requested: Optional[bool] = False,
) -> str:
"""
Constructs a GitHub Search Query
Expand All @@ -100,8 +104,12 @@ def _construct_query(
Examples
--------
type:pr author:user1
type:pr author:user2 updated:<=2021-01-01
type:pr author:user1 author:user2 updated:2021-01-01..2021-03-01
type:pr author:user2 created:<=2021-01-01
type:pr author:user1 author:user2 created:2021-01-01..2021-03-01
type:pr author:user2 updated:>=2021-01-01
type:pr involves:user2
type:pr reviewed-by:user1
type:pr review-requested:user1
All dates are inclusive.
See GitHub Docs for full options https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests
Expand All @@ -112,12 +120,16 @@ def _construct_query(
author_filter = 'author'
if use_involves:
author_filter = 'involves'
elif use_reviewed_by:
author_filter = 'reviewed-by'
elif use_review_requested:
author_filter = 'review-requested'

logger.debug('using author filter %s', author_filter)

for u in users:
query.append(f'{author_filter}:{u}')

logger.debug('using author filter %s', author_filter)

time_filter = 'created'
if use_updated:
time_filter = 'updated'
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ classifiers = [
python = "^3.8"
pygithub = "^1.58.1"
click = "^8.1.3"
cloup = "^2.1.0"
pandas = "^2.0.1"
pyarrow = "^11.0.0"
rich = "^13.3.5"
Expand Down
16 changes: 16 additions & 0 deletions tests/collectors/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,22 @@
'type:pr involves:user1 updated:2009-01-01..2010-01-01',
id='involves_updated_before_and_after_returned_results',
),
pytest.param
(
('user1', 'user2'),
{ 'use_reviewed_by': True },
[ _mock_issue1, _mock_issue2 ],
'type:pr reviewed-by:user1 reviewed-by:user2',
id='reviewed_by_user',
),
pytest.param
(
('user1', 'user2'),
{ 'use_review_requested': True },
[ _mock_issue1, _mock_issue2 ],
'type:pr review-requested:user1 review-requested:user2',
id='review_requested_user',
),
])
@patch('prfiesta.collectors.github.Github')
def test_collect(
Expand Down
53 changes: 45 additions & 8 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,44 +19,65 @@ def test_main_missing_users() -> None:
assert "Missing option '-u' / '--users'" in result.output
assert result.exit_code == FAILURE_CODE


@pytest.mark.parametrize('arguments', [
pytest.param(['--users', 'user', '--use-involves', '--use-reviewed-by', '--use-review-requested']),
pytest.param(['--users', 'user', '--use-involves', '--use-reviewed-by']),
pytest.param(['--users', 'user', '--use-reviewed-by', '--use-review-requested']),
pytest.param(['--users', 'user', '--use-involves', '--use-review-requested']),
])
def test_main_author_filters_mutually_exclusive(arguments: List[str]) -> None:
runner = CliRunner()
result = runner.invoke(main, arguments)

assert 'the following parameters are mutually exclusive' in result.output
assert result.exit_code == FAILURE_CODE

@pytest.mark.parametrize(('params', 'expected_collect_params', 'collect_response', 'expected_output_type'), [
pytest.param
(
['--users', 'test_user'],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False)],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='user_provided',
),
pytest.param
(
['--users', 'test_user', '--after', '2020-01-01'],
[call('test_user', after=datetime(2020, 1, 1), before=None, use_updated=False, use_involves=False)],
[call('test_user', after=datetime(2020, 1, 1), before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='with_after',
),
pytest.param
(
['--users', 'test_user', '--before', '2020-01-01'],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=False)],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='with_before',
),
pytest.param
(
['--users', 'test_user', '--before', '2020-01-01', '--after', '2009-01-01'],
[call('test_user', before=datetime(2020, 1, 1), after=datetime(2009, 1, 1), use_updated=False, use_involves=False)],
[call(
'test_user',
before=datetime(2020, 1, 1),
after=datetime(2009, 1, 1),
use_updated=False,
use_involves=False,
use_reviewed_by=False,
use_review_requested=False)],
pd.DataFrame(),
'csv',
id='with_before_and_after',
),
pytest.param
(
['--users', 'test_user'],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False)],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(
data=[(1, 2, 3)],
columns=['col1', 'col2', 'col3'],
Expand All @@ -67,7 +88,7 @@ def test_main_missing_users() -> None:
pytest.param
(
['--users', 'test_user', '--output-type', 'parquet'],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False)],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(
data=[(1, 2, 3)],
columns=['col1', 'col2', 'col3'],
Expand All @@ -78,19 +99,35 @@ def test_main_missing_users() -> None:
pytest.param
(
['--users', 'test_user', '--before', '2020-01-01', '--use-updated'],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=True, use_involves=False)],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=True, use_involves=False, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='with_use_updated',
),
pytest.param
(
['--users', 'test_user', '--before', '2020-01-01', '--use-involves'],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=True)],
[call('test_user', before=datetime(2020, 1, 1), after=None, use_updated=False, use_involves=True, use_reviewed_by=False, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='with_use_involves',
),
pytest.param
(
['--users', 'test_user', '--use-reviewed-by'],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=True, use_review_requested=False)],
pd.DataFrame(),
'csv',
id='user_reviewed_by',
),
pytest.param
(
['--users', 'test_user', '--use-review-requested'],
[call('test_user', after=None, before=None, use_updated=False, use_involves=False, use_reviewed_by=False, use_review_requested=True)],
pd.DataFrame(),
'csv',
id='user_review_requested',
),
])
@patch('prfiesta.__main__.Spinner')
@patch('prfiesta.__main__.Live')
Expand Down

0 comments on commit b451bd6

Please sign in to comment.