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.

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

DOC: update contributing to pandas documentation #48113

Merged
merged 3 commits into from
Sep 1, 2022

Conversation

marenwestermann
Copy link
Contributor

@marenwestermann marenwestermann commented Aug 16, 2022

I removed the part in the Committing your code section where it is said that commit messages should include the number of the GitHub issue the commit is contributing towards. I updated the pull request template to include cross references so that it's more clear how people should make use of referencing GitHub issues.

I fixed the code snippet git commit -m which causes an error and updated the documentation to include both commit message versions, the one line commit message and the more detailed commit message that can be done with an editor. Let me know if you would like to keep both versions or would like further changes.

Additionally, I moved the section about the prefixes to the Finally, make the pull request section of the documentation. Let me know if the prefixes need to be updated.

@@ -1,4 +1,5 @@
- [ ] closes #xxxx (Replace xxxx with the Github issue number)
- [ ] xref #xxxx (Replace xxxx with the Github issue number)
Copy link
Member

@mroeschke mroeschke Aug 16, 2022

Choose a reason for hiding this comment

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

Why was this added? I am concerned that it may be confusing with there also being closes #xxxx above

IMO we want to encourage the users to use closes #xxxx if applicable so it automatically closes the linked issue

Copy link
Member

Choose a reason for hiding this comment

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

Yeah agreed. There are already many checkboxes you have to tick, would prefer to not add this

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

Thanks @marenwestermann, nice update. Added couple of comments of things that I'd change. In the current docs, your changes look fine other than the template changes already mentioned.

relevant GitHub issues in your commit message using GH1234 or #1234. Either style
is fine, but the former is generally preferred:
Finally, commit your changes to your local repository with an explanatory commit
message of ``< 80`` chars::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
message of ``< 80`` chars::
message::

I know this already exists now, but since we squash commits, not very important, and we can simplify things a bit.

Comment on lines 202 to 207
Alternatively, you can just type ``git commit`` which opens an editor and use the
following commit message structure:

* a subject line with ``< 80`` chars.
* One blank line.
* Optionally, a commit message body.
Copy link
Member

Choose a reason for hiding this comment

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

I'd just delete all this.

@marenwestermann
Copy link
Contributor Author

marenwestermann commented Aug 22, 2022

Thanks for your feedback! I addressed your comments. I previously added the line - [ ] xref #xxxx (Replace xxxx with the Github issue number) in PULL_REQUEST_TEMPLATE.md because sometimes a pull request contributes to an issue but doesn't close it such as here: #44798. However, I understand your concerns and removed this line.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @marenwestermann, nice improvement.

@mroeschke mroeschke added this to the 1.6 milestone Sep 1, 2022
@mroeschke mroeschke merged commit b1c3da6 into pandas-dev:main Sep 1, 2022
@mroeschke
Copy link
Member

Thanks @marenwestermann!

@marenwestermann marenwestermann deleted the committing-your-code branch September 2, 2022 09:39
@mroeschke mroeschke modified the milestones: 1.6, 2.0 Oct 13, 2022
noatamir pushed a commit to noatamir/pandas that referenced this pull request Nov 9, 2022
* update contributing to pandas documentation

* address requested changes

* fix typo
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: "Committing your code" in "Contributing to pandas" needs update
4 participants