Skip to content

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

Merged
merged 4 commits into from
Sep 20, 2017

Conversation

willingc
Copy link
Contributor

@willingc willingc commented Sep 18, 2017


- Improved docs for documentation contributors (:issue:`17579`)


Copy link
Member

@gfyoung gfyoung Sep 18, 2017

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.

Copy link
Contributor Author

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. 😄

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 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.

Copy link
Contributor Author

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.

Copy link
Member

@gfyoung gfyoung Sep 19, 2017

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.

Copy link
Contributor

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.

@gfyoung
Copy link
Member

gfyoung commented Sep 18, 2017

@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?

@gfyoung gfyoung added the Docs label Sep 18, 2017
@willingc
Copy link
Contributor Author

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. If you're not the developer type, contributing to the documentation is still of huge value.) I doubt that the author's intent was to split folks into "developer"/"not developer". It sends the message "even if you can't write code... the docs are still valuable".

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.

@gfyoung
Copy link
Member

gfyoung commented Sep 18, 2017

The existing wording reinforces the myth that documentation is not as valuable as code

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."

@willingc
Copy link
Contributor Author

That being said, I see no reason with modifying that one sentence (if that's your issue) instead of modifying the entire two paragraphs.

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.

@gfyoung
Copy link
Member

gfyoung commented Sep 19, 2017

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
Copy link

codecov bot commented Sep 19, 2017

Codecov Report

Merging #17580 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            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
Flag Coverage Δ
#multiple 88.99% <ø> (ø) ⬆️
#single 40.2% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.77% <0%> (-0.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37e23d0...029f25d. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 19, 2017

Codecov Report

Merging #17580 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            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
Flag Coverage Δ
#multiple 88.96% <ø> (-0.03%) ⬇️
#single 40.19% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/plotting/_tools.py 72.92% <0%> (-6.08%) ⬇️
pandas/plotting/_core.py 82.52% <0%> (-0.21%) ⬇️
pandas/core/frame.py 97.77% <0%> (-0.1%) ⬇️
pandas/core/indexes/multi.py 96.9% <0%> (ø) ⬆️
pandas/plotting/_converter.py 63.38% <0%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 37e23d0...8daa606. Read the comment docs.


- Improved docs for documentation contributors (:issue:`17579`)


Copy link
Contributor

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.

@gfyoung
Copy link
Member

gfyoung commented Sep 19, 2017

In the past that’s what we’ve done, but I’m open to logging doc changes here.

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.

@TomAugspurger
Copy link
Contributor

Sure, we can certainly discuss that, but perhaps better if we table for a broader discussion

This seems like an opportune place to discuss it. @jorisvandenbossche Thoughts?

My thoughts are

Pros:

  1. pixels are cheap. Having more items in the whatsnew isn't a bad thing
  2. Contributors get to see their work reflected in the release notes
  3. Emphasizes that we value doc contributions, they don't have special rules

Cons:

  1. The whatsnew is a common source of merge conflicts, which can be tough for new contributors (but we maintainers can handle them in the web UI with little difficulty, so not a big problem)

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.

@jreback
Copy link
Contributor

jreback commented Sep 19, 2017

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.

@jorisvandenbossche
Copy link
Member

+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").

@gfyoung
Copy link
Member

gfyoung commented Sep 20, 2017

@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).

@gfyoung
Copy link
Member

gfyoung commented Sep 20, 2017

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.

@jorisvandenbossche @TomAugspurger : I actually wasn't aware of this, but in light of this comment, is this change really a newish section?

@TomAugspurger
Copy link
Contributor

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.

@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 :)

@gfyoung
Copy link
Member

gfyoung commented Sep 20, 2017

I think we're discussing the merits of changing the informal policy of not having whatsnew notes.

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.

Let's not be constrained by past habits :)

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 whatsnew documentation. I'm not getting stuck in old habits, but I am hesitant to alter how we document changes so late in the release cycle (IINM, the end of the month is our goal).

@@ -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')
Copy link
Member

Choose a reason for hiding this comment

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

Hmm...what happened here?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Member

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. 😄

@willingc
Copy link
Contributor Author

willingc commented Sep 20, 2017

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.

@gfyoung
Copy link
Member

gfyoung commented Sep 20, 2017

Ok. I've made one last pass at this PR. I've remove the whatsnew change, but I hope you will reconsider your policy in a future PR.

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. 😄

@gfyoung
Copy link
Member

gfyoung commented Sep 20, 2017

@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.

@willingc
Copy link
Contributor Author

No problem @gfyoung. I'll open the issue. Thanks!

@TomAugspurger
Copy link
Contributor

Thanks @willingc!

@willingc willingc deleted the docs-welcome branch September 20, 2017 21:32
alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017
* 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
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Revise doc contribution intro to encourage contributors
5 participants