Show Github PRs on Trac

[dd32]

With Github PRs being used more often now, there's been an attempt by a few people to expose Github PRs within Trac, this ticket is being used for tracking that effort.

Initially this is going to be enabled for Core trac only, and for the WordPress/WordPress-develop Github repo only, but has the potential to have multiple repo's and multiple tracs in the future if needed.

Initial development of this was worked on by @andraganescu @desrosj @isabel_brison @noisysocks @pento @talldanwp

The commits that follow this have however been rewritten heavily by myself to make it performant and fit the WordPress.org style.

---

Task List: ( ✅Completed, ❌Not yet, ❓Pending Question)
✅Track PRs on Github, and show them on Trac Tickets
✅Add a mention to Tickets comments when a PR is opened.
✅Always load PRs, for authenticated and unauthenticated (Currently it only auto-loads for authenticated requests)
✅Sync PR Comments to Trac
✅ Switch to using a Github App OAuth Token from dd32's personal Github token get_authorization_token() (or change it to select a random users token..)
❌Sync PR Comment edits to Trac
❌Sync PR Code Review comments to Trac #5053
❌Sync PR Code Review comment edits to Trac
❓Should all Github PR comments trigger Trac Emails? (Note: Edits won't be able to, and responses to existing code review instances would be best done as edits too)
❓Should Github PR activity be piped to Trac Firehose channels to compensate for lack of edit emails?

[dd32]

In 9340:

Trac: Add initial code to show Github PRs on Trac tickets.

Props dd32, andraganescu, desrosj, isabel_brison, noisysocks, pento, talldanwp.
See #4903.

[dd32]

In 9341:

Trac: Bump scripts version.

See #4903.

[dd32]

The Core Trac caches are slowly clearing.. ​#core43805 is an example of a ticket with a single PR, and ​#core48502 for 3 PRs.

[Mte90]

Hi sorry for the questions, but I have few of them because this is implementation is revolutionary:

• When the handbook will be updated? In this way we can suggest an alternative and more easy way instead of doing patches also at contributors day
• This pr will be tracked inside Trac in the My Patches? I think that this require https://meta.trac.wordpress.org/ticket/4447 enabled for everyone
• I think that also few guidelines about how to do the pr in the handbook will be very helpful, I think that for the next patch I will do I will switch to GitHub.

[dd32]

Hi @Mte90 and thanks for asking

When the handbook will be updated? In this way we can suggest an alternative and more easy way instead of doing patches also at contributors day

It's up to the Core team as to when that happens, there's a few more things in the works that a few believe is needed before full recommendation of it.

This pr will be tracked inside Trac in the My Patches?

At this point, there's no plans for that. Unfortunately integrating with ​https://core.trac.wordpress.org/my-patches would require some significant changes to how the Trac query happens.
There's the option that we could potentially just include an extra list below the table of any open PRs, but that's not high on my list of priorities for now.

There's a more-or-less hidden (just unannounced) feature in profiles - you can link your Github account with your WordPress.org account which helps #4447 greatly.

I think that also few guidelines about how to do the pr in the handbook will be very helpful

Once again, that's up to the Core team to update their handbooks - AFAIK there's no real current guidelines, but I expect that would happen pretty quickly if PR quality becomes an issue.

[dd32]

In 9365:

Trac: Github PRs: Properly craft the Github API request when a POST request is used, and ensure that we set the headers correctly.

See #4903.

[dd32]

In 9366:

Trac: Github PRs: When a PR is first detected referencing a ticket, add an inline mention to Trac.

For now, this doesn't trigger email notifications.

See #4903.

[dd32]

In 9367:

Trac: Github PRs: style prbot comments similar to slackbot mentions when the comment is italic (which signifies it's an inline comment, rather than a regular comment).

See #4903.

[dd32]

See also [9356] which still hasn't actually been deployed

Trac: Increase the padding of the 'chat-bot' comments (slackbot, etc) by 1px to hide the underline from the parent element.
This also hides the comment action buttons since they're not useful here.

[dd32]

In 9368:

Trac: Bump scripts version again to deploy r9356.

See #4903.

[dd32]

In 9370:

Trac: Github PRs: Add a wrapper to call the trac wpapi endpoints.

See #4903.

[dd32]

In 9371:

Trac: Github PRs: Fix a typo preventing finding the WordPress.org user.

See #4903.

[dd32]

In 9372:

Trac: Github PRs: Add a wrapper for the Trac class for easier testing.

See #4903.

[dd32]

Adding a basic task list to the ticket description to track current and future parts of this ticket that's needed.

[dd32]

In 9510:

Trac: Expand the Github PR section by default for all users, not just authenticated ones.

See #4903.

[dd32]

In 9511:

Trac: Update the 'no PRs found' text for the Github PRs on Trac.

Props noisysocks.
See #4903.
Fixes #5042.

[dd32]

In 9513:

Trac: Bump scripts version.

See #4903, #5042.

[dd32]

In 9514:

Trac: Remove the no-longer-used PRNumber trac field.

Props noisysocks.
See #4903.

[desrosj]

Some related changes elsewhere:

• Initial feature has been announced: ​https://make.wordpress.org/core/2020/02/21/working-on-trac-tickets-using-github-pull-requests/
• New page in the handbook: ​https://make.wordpress.org/core/2020/02/21/working-on-trac-tickets-using-github-pull-requests/
• Updated the paragraph about PRs on the Contribute with Git page: ​https://make.wordpress.org/core/handbook/contribute/git/#summary
• Created and committed an initial pull request template to properly set expectations about PRs to the repo: ​https://core.trac.wordpress.org/ticket/49489

[dd32]

See also: #5052, #5052, #5054

[ocean90]

I think the display of mergeable_state needs some improvement. When a build is still running the value is unstable which is incorrectly displayed as "❌ Failing tests". When tests are actually failing (​example) the state is unknown and displayed as "undefined".

It seems like ​https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref will give us the correct results for tests.

[dd32]

In 9534:

Trac: Github PRs: Use the Github .diff rather than .patch as they can differ in subtle ways.

Props azaozz, noisysocks.
See #4903.

[dd32]

Replying to ocean90:

I think the display of mergeable_state needs some improvement. When a build is still running the value is unstable which is incorrectly displayed as "❌ Failing tests". When tests are actually failing (​example) the state is unknown and displayed as "undefined".

Agreed, The logic is in the prStatus() function in the JS, if someone could properly list out the actual statuses that we need to take into account. I just copied what the previous implementation had used, which evidently doesn't cover all bases.

It seems like ​https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref will give us the correct results for tests.

PR's don't link to that endpoint directly, but crafting the URL for it seems to work.
The only "issue" I see is that the output only ever mentions AppVeyor, not Travis.. I'm unsure if that's just because it's the slower of the two, but the Github docs seem to suggest it should include both? (It's github.. it probably works)
eg. ​pulls/166 links to ​/statuses/{SHA} which can be altered to a ​{SHA}/status request.

[dd32]

In 9564:

Trac: Github PRs: Switch to using a Github App Token for authentication with Github.

This significantly increases the number of API requests that can be made per hour, and enables access to several additional API endpoints.

This uses the MIT licensed ​https://github.com/adhocore/php-jwt to generate the JWTs as required by the Github App API.

See #4903, #5052.

[garrett-eclipse]

@dd32 using the feature it would be nice if grunt patch:XXXX would list not only uploaded .diff patches but also PRs so from the list a user could choose to patch using either.
(I can open a new ticket for this)

This would simplify the patching process as you wouldn't need to find the PR URL and apply it as follows;
grunt patch:http://github.../wordpress-develop/pull/<id>

As well it would avoid situations where there's some svn diffs but a newer PR and users used to running grunt patch don't overlook the fact there's a PR that's actually newer than any of the diffs that'd be listed by the grunt command.

P.S. Awesome work here, thanks

[dd32]

Replying to garrett-eclipse:

@dd32 using the feature it would be nice if grunt patch:XXXX would list not only uploaded .diff patches but also PRs so from the list a user could choose to patch using either.

You can already pass a PR url to grunt:patch and it'll work apparently.

(I can open a new ticket for this)

The best place would be to file an issue on ​WordPress/grunt-patch-wordpress.

[garrett-eclipse]

Thanks @dd32

Opened a ticket for that idea here;
​https://github.com/WordPress/grunt-patch-wordpress/issues/83

This brings the recent addition of GitHub PR support on core.trac.wordpress.org to Meta.trac.wordpress.org

This is part of https://meta.trac.wordpress.org/ticket/4903

For more information on how this is supposed to work, check out ​https://make.wordpress.org/core/2020/02/21/working-on-trac-tickets-using-github-pull-requests/

[dd32]

In 9695:

Trac: Enable GitHub PR support for Meta Trac.

See ​https://github.com/WordPress/wordpress.org/pull/4.
See #4903.

Comitted in https://meta.trac.wordpress.org/changeset/9695

This change causes the Travis CI status to be queried directly for determining the PR state, rather than relying upon the PR state variables.

Strangely the mergeable_state field is showing blocked for some PRs, where the Web and curl requests show clean.. After reviewing the PRs with that state, the new Travis checks will catch those otherwise they appear to be clean.

Part of https://meta.trac.wordpress.org/ticket/4903

[coffee2code]

Replying to dd32:

Trac: Enable GitHub PR support for Meta Trac.

I've noticed that @otto42, @ryelle, and myself (at least amongst recent committers) don't have our commits associated with our GitHub accounts. Anything we'd need to do to make that connection? My username and email address is the same on both services.

[ocean90]

Replying to coffee2code:

Anything we'd need to do to make that connection? My username and email address is the same on both services.

You only have to add your-username@git.wordpress.org to ​https://github.com/settings/emails, the email doesn't need to be verified.

[Otto42]

That worked, can confirm.

[coffee2code]

Excellent. Thanks!

[dd32]

I'm going to close this as fixed, as there's a number of follow up tickets for individual enhancements for it.

[dd32]

In 9897:

Trac: When pulling GitHub PR status, check the Test Runners results directly to avoid misleading/incorrect/missing PR statuses.

Merges ​https://github.com/WordPress/wordpress.org/pull/5
See #4903.

[dd32]

In 9899:

Trac: Bump scripts version after r9897 and r9898

See #5052, #4903.