-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Add a more welcoming tone for new contributors #17580
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
Conversation
doc/source/whatsnew/v0.21.0.txt
Outdated
|
||
- Improved docs for documentation contributors (:issue:`17579`) | ||
|
||
|
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.
Not necessary in this case. Generally, documentation changes don't get a whatsnew
.
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.
Why not? They're a contribution, and it demonstrates the project values a documentation contributor. 😄
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.
I think our documentation speaks for itself when it comes to how much we value it. 😄
That being said, our change-logs are generally for functional changes / bug fixes.
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.
@gfyoung I believe that you value it 😄
My point is that the cost of putting documentation changes at the end of the what's new are outweighed by the benefits of encouraging contributors to the documentation.
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.
@willingc : Still would like to be removed. I understand where you're coming from, but we generally don't log documentation changes, just functional changes / bug fixes.
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.
In the past that’s what we’ve done, but I’m open to logging doc changes here. I could imagine someone reading the change log and seeing that a section has been rewritten and clicking through to see what’s changed.
@willingc : Thanks for the PR! Reading this, I'm a little confused by which section of the docs is scaring away new contributors. In fact, if anything, it's scaring away current contributors from writing new docs because it might come across as too technical and less understandable for the general user. Could you clarify what you found "less friendly" about the existing documentation? |
The existing wording reinforces the myth that documentation is not as valuable as code. (i.e. Just to clarify my thoughts...I think the pandas contributors and documentation are wonderful. Please consider my comments/suggestions as something that I have found in other projects to improve open source documentation and strengthen the contributor community. |
Ah, I see. So we're definitely trying to not scare away non-devs. We explicitly refer non-developers because we've had several people file issues who then back away from contributing because they don't believe they're "dev-like" and therefore can't contribute. That being said, I see no reason with modifying that one sentence (if that's your issue) instead of modifying the entire two paragraphs. How about this: "If you're not the developer type, contributing to the documentation is equally as valuable to us." |
I modified both paragraphs to create an affirmative tone and economy of expression. I'm happy to just modify the one sentence if that is what you prefer. Just let me know. |
Compared to the original, the modifications dampen the enthusiasm that the author originally intended with these paragraphs. Therefore, I would prefer to modify that one sentence, and this PR will be on its way. |
Codecov Report
@@ Coverage Diff @@
## master #17580 +/- ##
==========================================
- Coverage 91.22% 91.2% -0.02%
==========================================
Files 163 163
Lines 49625 49625
==========================================
- Hits 45270 45261 -9
- Misses 4355 4364 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17580 +/- ##
==========================================
- Coverage 91.22% 91.18% -0.05%
==========================================
Files 163 163
Lines 49625 49626 +1
==========================================
- Hits 45270 45249 -21
- Misses 4355 4377 +22
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.21.0.txt
Outdated
|
||
- Improved docs for documentation contributors (:issue:`17579`) | ||
|
||
|
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.
In the past that’s what we’ve done, but I’m open to logging doc changes here. I could imagine someone reading the change log and seeing that a section has been rewritten and clicking through to see what’s changed.
Sure, we can certainly discuss that, but perhaps better if we table for a broader discussion, when the original focus was just to improve the contributing docs. |
This seems like an opportune place to discuss it. @jorisvandenbossche Thoughts? My thoughts are Pros:
Cons:
Overall a 👍 from me @willingc We're aware of the Travis failure (new release of Pyarrow broke a test), so no need to worry about that. |
this PR is fine. As far as what'snew for documentation, we generally don't do this unless its a newish section, e.g. something we would want to point users to. |
+1 from me as well. @TomAugspurger gave a good summary. BTW, about the actual PR. @willingc I find your original rewrite of the first sentence better (which left out the "If you're not the developer type"). |
@jorisvandenbossche : I actually liked that part because it directly addresses those who actually don't believe that they are (people say that explicitly as reasons for being wary of contributing). |
@jorisvandenbossche @TomAugspurger : I actually wasn't aware of this, but in light of this comment, is this change really a newish section? |
I think we're discussing the merits of changing the informal policy of not having whatsnew notes. Let's not be constrained by past habits :) |
Right, but I'll bring up my question again: why in this PR? Something like this is better suited for an issue that the original changes are getting lost in a larger discussion.
I might add that a decision like this, while not necessarily as heavy as say an API change does have a noticeable impact on what we share in the |
@@ -389,7 +389,7 @@ New Behavior: | |||
--------------------------------------------------------------------------- | |||
ValueError: Of the three parameters: start, end, and periods, exactly two must be specified | |||
|
|||
In [3]: pd.period_range(start='2017Q1', end='2017Q4', periods=6, freq='Q') |
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.
Hmm...what happened here?
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.
Atom stripped the trailing whitespace.
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.
I can add it back if you want.
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.
No, make the change by all means! If this was a Python file, flake8
would have been unhappy. 😄
Ok. I've made one last pass at this PR. I've removed the whatsnew change, but I hope you will reconsider your policy in a future PR. I've put my original first sentence back and made the "you don't need to be a developer" the second sentence. Feel free to make any additional changes by editing the PR directly. Thanks. |
Absolutely. I think it's safe to say that you don't really need to hope, as we're reconsidering it right now in your PR, for better or worse. 😄 |
@willingc : Actually, why don't you open an issue for this (I'm on my phone, so hard to do). Then we can transfer the conversation there and merge this in. |
No problem @gfyoung. I'll open the issue. Thanks! |
Thanks @willingc! |
* Add a more welcoming tone for new contributors * create doc section in whatsnew * Edit per @gfyoung review * Edit first 2 sentences and revert whatsnew change
* Add a more welcoming tone for new contributors * create doc section in whatsnew * Edit per @gfyoung review * Edit first 2 sentences and revert whatsnew change
git diff upstream/master -u -- "*.py" | flake8 --diff