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

doc: add config for welcome bot #21074

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# https://github.com/behaviorbot/welcome
# combines new-issue-welcome, new-pr-welcome, and first-pr-merge

# Comment on first-time issues
# https://github.com/behaviorbot/new-issue-welcome
newIssueWelcomeComment: >
👋 Thanks for opening your first issue here! If you're reporting a bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

To make it easier for us to investigate your issue, please follow the [contributing guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/issues.md).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to list common wrong repo cases here (e.g. npm, v8, node-gyp, .etc) but we can do that in a future PR.


# Comment on PRs from first-time contributors
# https://github.com/behaviorbot/new-pr-welcome
newPRWelcomeComment: >
💚 Thanks for opening this pull request! 💚

Please make sure to update documentation and tests when appropriate, and follow the [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)

For non-trivial changes, pull requests must be left open for *at least* 48 hours during the week, and 72 hours on a weekend (in UTC time) before being merged.

If this pull request changes includes code changes, the CI must be started by a [collaborator](https://github.com/nodejs/node/blob/master/GOVERNANCE.md#collaborators).

We get a lot of pull requests on this repo, so please be patient and we will review this as soon as we can.

# Comment when a first-time contributor's PR is merged
# https://github.com/behaviorbot/first-pr-merge
firstPRMergeComment: >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed since we rarely use the merge button and have our own workflow. It can be added when we adopt something else (e.g. a Jenkins job that merge the PR for us)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should leave it in for cases when the merge button is used?

Copy link
Member

@joyeecheung joyeecheung Jun 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zeke I am fine with leaving those texts. Although just for the sake of explanation, I am not actually aware of anybody using the button...at least not for first-time PRs. If you look at the PRs that are "merged", basically they are all opened and merged by collaborators, because right now if you don't use the merge button, you will need to push the HEAD that you are about to push to master to the same PR branch in order to make it purple. Unless you are the person that's going to push to master real soon, you are unlikely to get to the final state of that commit and push it to the PR branch right before it's pushed to master.

https://github.com/nodejs/node/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Amerged

Congratulations on merging your first pull request! 👍
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be triggered even if we don't merge the GitHub way?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this app purely uses GitHub Apps to listen on webhook events, like the PR being closed and then checks if the PR was merged and if it was that individual's first PR to be merged. So it depends on GitHub's merged property to be true for the PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We often don't have that...