Success

User tests: Successful: Unsuccessful:

avatar alikon
alikon
21 Jul 2018

Pull Request for Issue #210.

Summary of Changes

add a new option to filter PR's only from a branch

Additional comments

maybe should be better to properly use the Github API

avatar alikon alikon - open - 21 Jul 2018
avatar alikon alikon - change - 21 Jul 2018
The description was changed
avatar alikon alikon - edited - 21 Jul 2018
avatar mbabker
mbabker - comment - 21 Jul 2018

OK, looking at this again, what is really being gained here? We don't limit the number of API calls to GitHub and there is already a display filter for branches in the current code.

465e2ef 21 Jul 2018 avatar lang
avatar alikon
alikon - comment - 21 Jul 2018

yes, it should be better fetched directly from the github API

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 21 Jul 2018

@mbabker to download what is needed.

Test 3.8, 3.9, 4.0 on different Installation and download same PR on each Installation.

avatar mbabker
mbabker - comment - 21 Jul 2018

Without changing the lookup mechanism to use GitHub's GraphQL API instead of the REST API (which inherently removes the ability to pull data without requiring GitHub credentials in the component), we can't optimize the API lookups. So to me, without the ability to do that, this is just an artificial filter to not save some data to the database. To me the only improvement is it saves you from having to use the "Filter Target Branch" UI filter.

avatar alikon
alikon - comment - 21 Jul 2018

correct

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 21 Jul 2018

so you can't avoid downloading unnecessary PR?

avatar mbabker
mbabker - comment - 21 Jul 2018

Right now, no, there is no way to avoid downloading extra data. There is a way to change the API query, but as I said it has the side effect of removing the ability to fetch data without saving your GitHub credentials to the component configuration. I don't want to remove that feature right now.

avatar alikon
alikon - comment - 21 Jul 2018

as stated, without changing the github lookup no, with this pr we can only avoid to store "unnecessary" pr's on db

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 21 Jul 2018

got it, thanks Guys for Info. So its easiest to stay as is.

avatar alikon alikon - change - 9 Sep 2018
Status New Closed
Closed_Date 0000-00-00 00:00:00 2018-09-09 09:39:26
Closed_By alikon
avatar alikon alikon - close - 9 Sep 2018
avatar alikon alikon - close - 9 Sep 2018
avatar alikon alikon - head_ref_deleted - 9 Sep 2018

Add a Comment

Login with GitHub to post a comment