Skip to content

Improving Contributing guidlines #2260

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
Merged
rcurtin merged 5 commits into from
Mar 13, 2020
Merged

Improving Contributing guidlines #2260

rcurtin merged 5 commits into from
Mar 13, 2020

Conversation

@jeffin143
Copy link
Member

@jeffin143 jeffin143 commented Mar 6, 2020

This is in view of messages being received by new contributors in irc/slack channels to review PR, I thought of addressing it, I have tried my best not to sound rude.

If anyone finds this rude or as of that matter find that it is not necessary to mention explicitly and can be handled internally , I will go ahead and close it.

I really do appreciate any formatting of sentence to make it more warm and welcoming ❤️

kartikdutt18
kartikdutt18 reviewed Mar 6, 2020
View reviewed changes
Copy link
Member

@kartikdutt18 kartikdutt18 left a comment

Choose a reason for hiding this comment

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

Hi @jeffin143, Thanks for adding this to the guidelines. Just one small thing, other than that this looks fine. Could you please take a look.
Thanks.

@favre49
Copy link
Member

favre49 commented Mar 6, 2020

I agree with the spirit of the addition, but I feel like a lot of it is redundant and wordy. Do you think we could do with something along the lines of:

Since members of the Contributors team only work on mlpack in their free time, it may take some time for them to review pull requests. While gentle reminders are welcome, please be patient and avoid constantly messaging the contributors.

As for the section on reviewing pull requests, I feel like that belongs in the Code of Conduct, especially since this document pertains to informing new contributors. Let me know what you think :)

@jeffin143
Copy link
Member Author

jeffin143 commented Mar 6, 2020

I agree with the spirit of the addition, but I feel like a lot of it is redundant and wordy. Do you think we could do with something along the lines of:

Since members of the Contributors team only work on mlpack in their free time, it may take some time for them to review pull requests. While gentle reminders are welcome, please be patient and avoid constantly messaging the contributors.

As for the section on reviewing pull requests, I feel like that belongs in the Code of Conduct, especially since this document pertains to informing new contributors. Let me know what you think :)

Yes that's true , but I was just caustios enough that if I directly quote something like that , it would be sounded as rude so I just took a better approach of describing what the issue

But yours truly is nice , I will just change it accordingly and then may be move ahead with others suggestions too

But I definitely agree it is somewhat redudant

rcurtin
rcurtin reviewed Mar 9, 2020
View reviewed changes
Copy link
Member

@rcurtin rcurtin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @jeffin143, I think it's really awesome that you've taken the time to do this. There is a human side to every open source project (after all, we are people, it's not just code), and I think that a lot that you've written here is really helpful in getting some attention on the human side of mlpack. :) I added some suggestions in the review comments; let me know what you think.

Also, one little thing, could you make the lines 80 characters? 😄

Thanks! 🚀

@jeffin143 jeffin143 force-pushed the contrib-guide branch 3 times, most recently from a62f7e4 to eae305f Compare March 10, 2020 06:45
@jeffin143
Copy link
Member Author

jeffin143 commented Mar 10, 2020 •
edited
Loading

There is a human side to every open source project (after all, we are people, it's not just code)

So true, and that is why I went ahead and did this 😄
Thanks for all the comments

ps : ignore previous edits, just found community.html having the involving section :), Might have missed it

zoq
zoq reviewed Mar 10, 2020
View reviewed changes
rcurtin
rcurtin reviewed Mar 11, 2020
View reviewed changes
Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Thanks, just added some more comments. From my side it's looking pretty good. 👍

zoq
zoq approved these changes Mar 12, 2020
View reviewed changes
Copy link
Member

@zoq zoq left a comment

Choose a reason for hiding this comment

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

Thanks for improving the contribution guidelines, not further comments from my side.

favre49
favre49 reviewed Mar 13, 2020
View reviewed changes
rcurtin
rcurtin approved these changes Mar 13, 2020
View reviewed changes
Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Awesome work @jeffin143, thanks for taking the time to make these improvements. 👍

@rcurtin rcurtin merged commit 11d4786 into mlpack:master Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rcurtin rcurtin rcurtin approved these changes

@kartikdutt18 kartikdutt18 kartikdutt18 left review comments

@favre49 favre49 favre49 left review comments

@zoq zoq zoq approved these changes

@birm birm birm approved these changes

Assignees

No one assigned

Labels

c: documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jeffin143 @favre49 @rcurtin @zoq @birm @kartikdutt18