-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add membership and commit rights document #12
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just asking some clarification on the necessity to become a member first before being given write priviledges.
Also, there are some dots missing at the end of sentences in Removing permissions
Thanks @guillaumeeb and @stsievert for the review |
If there are no further comments then I suggest that we merge this in 48 hours. All current owners have been informed of this PR and the intent to merge. |
+1 to merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this Matt. Generally this seems good. Had a couple questions.
Co-Authored-By: mrocklin <mrocklin@gmail.com>
Ideally everyone that is fairly actively involved in a project should be on board with a new addition.
Have pushed a few changes to address things raised here. Please let me know your thoughts on these changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do another 48-hour call for +/-1 on this. If there aren't any additional concerns then we can merge.
+1 on all of this from me |
+1 here |
Merging in 24 hours if there are no further comments |
Thanks all! 😄 |
This was previously discussed in dask/dask#3223