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.

Sign up for GitHub

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

Jump to bottom

Closes #1580: Update CONTRIBUTING.md #1581

Merged
merged 2 commits into from
Jul 14, 2022
Merged

Closes #1580: Update CONTRIBUTING.md #1581

merged 2 commits into from
Jul 14, 2022

Conversation

stress-tess
Copy link
Member

This PR (Closes #1580):

  • Adds Reviewing Pull Requests section and outlines expectations around resolving conversations
  • Adds expectations of posting in slack prior to merging a PR

jeichert60
jeichert60 approved these changes Jul 13, 2022
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
mhmerrill
mhmerrill approved these changes Jul 13, 2022
View reviewed changes
Copy link
Contributor

@mhmerrill mhmerrill left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@stress-tess stress-tess force-pushed the 1580_Update_CONTRIBUTING.md branch from cdf86d4 to 29d467b Compare July 13, 2022 14:22
Ethan-DeBandi99
Ethan-DeBandi99 approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@Ethan-DeBandi99 Ethan-DeBandi99 left a comment

Choose a reason for hiding this comment

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

Looks good. I am a bit confused about the need to post in slack. That seems to double up on tracking that github is already doing, but I might have missed a conversation somewhere detailing this.

@stress-tess
Copy link
Member Author

Looks good. I am a bit confused about the need to post in slack. That seems to double up on tracking that github is already doing, but I might have missed a conversation somewhere detailing this.

Yeah there was a conversation where @mhmerrill expressed a desire to have more awareness when PRs are merged. I also feel like this is a tad excessive/could get annoying when lots of smaller PRs are getting merged. Perhaps a middle ground here is to always add mike as reviewer? That way github will automatically email when things are merged

@mhmerrill
Copy link
Contributor

Looks good. I am a bit confused about the need to post in slack. That seems to double up on tracking that github is already doing, but I might have missed a conversation somewhere detailing this.

Yeah there was a conversation where @mhmerrill expressed a desire to have more awareness when PRs are merged. I also feel like this is a tad excessive/could get annoying when lots of smaller PRs are getting merged. Perhaps a middle ground here is to always add mike as reviewer? That way github will automatically email when things are merged

i am ok with this suggestion.

mhmerrill
mhmerrill reviewed Jul 14, 2022
View reviewed changes
CONTRIBUTING.md Outdated Show resolved Hide resolved
@Ethan-DeBandi99
Copy link
Contributor

Ethan-DeBandi99 commented Jul 14, 2022 •
edited by mhmerrill
Loading

@pierce314159 - should we update to require adding @mhmerrill to each issue as a requirement in the CONTRIBUTING.md instead of posting to slack.

I am ok with this to a point but maybe it should be a bit more generic like a senior/experienced member of the dev team...

Pierce Hayes added 2 commits July 14, 2022 13:46
Closes Bears-R-Us#1580: Update CONTRIBUTING.md
b10b841 
This PR (Closes Bears-R-Us#1580):
- Adds `Reviewing PR` section and outlines expectations around resolving conversations
- Adds expectations of posting in slack prior to merging a PR
Updated PR in response to feedback
58f8b9e
@stress-tess stress-tess force-pushed the 1580_Update_CONTRIBUTING.md branch from 29d467b to 58f8b9e Compare July 14, 2022 17:53
@Ethan-DeBandi99 Ethan-DeBandi99 merged commit 3a520f2 into Bears-R-Us:master Jul 14, 2022
@stress-tess stress-tess deleted the 1580_Update_CONTRIBUTING.md branch July 14, 2022 18:42
jeichert60 pushed a commit to jeichert60/arkouda that referenced this pull request Jul 28, 2022
@stress-tess @jeichert60
Closes Bears-R-Us#1580: Update CONTRIBUTING.md (Bears-R-Us#1581)
3ad27f3 
* Closes Bears-R-Us#1580: Update `CONTRIBUTING.md`

This PR (Closes Bears-R-Us#1580):
- Adds `Reviewing PR` section and outlines expectations around resolving conversations
- Adds expectations of posting in slack prior to merging a PR

* Updated PR in response to feedback

Co-authored-by: Pierce Hayes <[email protected]>
jeichert60 pushed a commit to jeichert60/arkouda that referenced this pull request Jul 28, 2022
@stress-tess @jeichert60
Closes Bears-R-Us#1580: Update CONTRIBUTING.md (Bears-R-Us#1581)
917dcf8 
* Closes Bears-R-Us#1580: Update `CONTRIBUTING.md`

This PR (Closes Bears-R-Us#1580):
- Adds `Reviewing PR` section and outlines expectations around resolving conversations
- Adds expectations of posting in slack prior to merging a PR

* Updated PR in response to feedback

Co-authored-by: Pierce Hayes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhmerrill mhmerrill mhmerrill approved these changes

@joshmarshall1 joshmarshall1 joshmarshall1 approved these changes

@jeichert60 jeichert60 jeichert60 approved these changes

@Ethan-DeBandi99 Ethan-DeBandi99 Ethan-DeBandi99 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Updates to CONTRIBUTING.md
5 participants
@stress-tess @mhmerrill @Ethan-DeBandi99 @jeichert60 @joshmarshall1