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

Adopting project workflow to new GitHub features #8554

Closed
Trott opened this issue Sep 15, 2016 · 9 comments
Closed

Adopting project workflow to new GitHub features #8554

Trott opened this issue Sep 15, 2016 · 9 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@Trott
Copy link
Member

Trott commented Sep 15, 2016

At the @nodejs/ctc meeting yesterday, @ChALkeR brought up the possibility of using some of the new GitHub tools and features to assist the workflow of landing code in this repository.

I'm particularly interested in whether the GitHub pull request approval feature might help us make things more robust. First, unlike leaving an "LGTM" comment, an approval gets removed if the code changes after the approval is made. I think that's a Good Thing (although others may find it irksome for small fixes perhaps?).

I wonder if approvals might make it reliable to use tools (like I believe @evanlucas has been working on) to generate the metadata for a pull request. I imagine searching comments for the string "LGTM" is imperfect. But it seems likely a list of approving collaborators can be obtained reliably from an API call.

I'm not sure what other features @ChALkeR or others might have in mind too. So, yeah, opening an issue for discussion. @nodejs/collaborators

@Trott Trott added the meta Issues and PRs related to the general management of the project. label Sep 15, 2016
@imyller
Copy link
Member

imyller commented Sep 15, 2016

I'm +1 for using new PR code review and approval features.

We can also comment the review approval with familiar "LGTM" for time being.

About API: looks like only early-access GitHub GraphQL API allows access to reviews and list of approving collaborators.

@Qard
Copy link
Member

Qard commented Sep 15, 2016

I'd agree that approval invalidation on code change is mostly a nice feature, but it might be a bit annoying to have to reapprove after asking that metadata get added to the commit. I'm still +1 though.

@targos
Copy link
Member

targos commented Sep 16, 2016

@Trott

an approval gets removed if the code changes after the approval is made.

I'm not sure that's true. I force pushed changes to #8552 and your approval is still there.

@jasnell
Copy link
Member

jasnell commented Sep 16, 2016

Big +1 on using the new tools.

@jbergstroem
Copy link
Member

One step closer to automating landing pull requests 🎉 I'm also for this but need to do some reading to better understand how we can use this from github-bot and other places. We're doing testing over at https://github.com/TestOrgPleaseIgnore if someone wants a playground.

@imyller
Copy link
Member

imyller commented Sep 16, 2016

Of the newly introduced features, the GitHub Projects could be used to better coordinate and track any efforts spanning across multiple issues and pull requests.

Currently many older & open issues remain forgotten and it's hard to keep up with the specific efforts overall progress.

https://help.github.com/articles/tracking-the-progress-of-your-work-with-projects/

Few examples:

  • WHATWG URL implementation that has multiple related issues open
  • async_wrap, async_hooks
  • Major V8 version upgrades (spanning multiple PRs)

@jbergstroem
Copy link
Member

Just want to reference #8675 here. As much as I like all new features introduced I can't help but feel that jumping on board now would be slightly premature. We still have to figure out scenarios where LGTM/"review approved" (or -1/denied) doesn't get properly zeroed on new commits to PR. It can also stall PR's with "pending reviews". These problems obviously outside of these features too, but I can't help feel like these features should be able to solve use cases like that.

@ChALkeR
Copy link
Member

ChALkeR commented Dec 8, 2016

New stuff: review requests for PRs and the search at the very top (Pull requests, Issues).

This is wonderful, as I have 6k+ of unread notifications and sometimes miss the ones that @mention me. It should be better now =).

@Trott
Copy link
Member Author

Trott commented Jul 15, 2017

This issue has been inactive for sufficiently long that it seems like perhaps it should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.

@Trott Trott closed this as completed Jul 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

7 participants