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

meta: investigate "incorporating feedback in your pull request" #23702

Closed
vsemozhetbyt opened this issue Oct 16, 2018 · 5 comments
Closed

meta: investigate "incorporating feedback in your pull request" #23702

vsemozhetbyt opened this issue Oct 16, 2018 · 5 comments
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.

Comments

@vsemozhetbyt
Copy link
Contributor

New GitHub feature:

https://help.github.com/articles/incorporating-feedback-in-your-pull-request/#applying-a-suggested-change

This can simplify nits addressing. We may need to update our contributing guides (including how-to steps to sync local branches with remote changes if needed).

@vsemozhetbyt vsemozhetbyt added meta Issues and PRs related to the general management of the project. doc Issues and PRs related to the documentations. discuss Issues opened for discussions and feedbacks. labels Oct 16, 2018
@vsemozhetbyt
Copy link
Contributor Author

Example: #23701 (comment)

@refack
Copy link
Contributor

refack commented Oct 16, 2018

Example2: https://github.com/nodejs/node/pull/22079/files#r225699740

IMHO it's a very nice feature for reviewers who are willing to make the extra effort.
We do need to be a little bit careful since GitHub is not an IDE, so when suggesting code changes it should be stated explicitly if it was tested, or is pseudocode.

@refack
Copy link
Contributor

refack commented Oct 16, 2018

We may need to update our contributing guides (including how-to steps to sync local branches with remote changes if needed).

I believe there's a balance to strike here. IMHO our contribution guide should focus on the things that are specific to work in this repo. While points that require general git and GitHub literacy should be referred to better guides. Such as the new https://lab.github.com/

@vsemozhetbyt
Copy link
Contributor Author

A doubt: technically, any Collaborator can assign themselves to a PR and accept an own proposed change. This can alleviate nit addressing for the PR author, but also can be felt like a bit arbitrarily thing.

@vsemozhetbyt
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

2 participants