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

Updated PULL_REQUEST_TEMPLATE.md #38696

Merged
merged 15 commits into from
Jan 4, 2021
Merged

Conversation

BobinMathew
Copy link
Contributor

@BobinMathew BobinMathew commented Dec 25, 2020

@BobinMathew
Copy link
Contributor Author

BobinMathew commented Dec 26, 2020

  • closes #38696
  • tests added / passed
  • passes pre-commit run --from-ref=upstream/master --to-ref=HEAD --all-files
  • whatsnew entry

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks @BobinMathew !

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Hey @BobinMathew - sorry to go back on this, but there's been some discussion in #38694 and it's been suggested to instead link to the docs and just have "Ensure all linting tests pass, see here how to run them" here

Do you want to update this in the template and also in the docs? Feel free to ask if you want/need help with any of this

@BobinMathew
Copy link
Contributor Author

Do you want to update this in the template and also in the docs? Feel free to ask if you want/need help with any of this

I would love to contribute 😁

@BobinMathew
Copy link
Contributor Author

it's been suggested to instead link to the docs and just have "Ensure all linting tests pass, see here how to run them" here

did you mean insert ?

@BobinMathew
Copy link
Contributor Author

  • closes #38696
  • tests added / passed
  • passes pre-commit run --from-ref=upstream/master --to-ref=HEAD --all-files
  • whatsnew entry
  • Ensure all linting tests pass, see here how to run them

Should the new PR include the above details ☝

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks @BobinMathew for updating! 👍 Just have some requests

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks @BobinMathew , we're almost there

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
doc/source/development/contributing.rst Outdated Show resolved Hide resolved
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks @BobinMathew !

@jorisvandenbossche any comments?

@BobinMathew
Copy link
Contributor Author

BobinMathew commented Jan 3, 2021

@MarcoGorelli
what should be the documentation problem for the new issue mentioned above

@BobinMathew
Copy link
Contributor Author

BobinMathew commented Jan 3, 2021

DOC: update contributing.rst

Location of the documentation

doc/source/development/contributing.rst

Documentation problem

Need additional information about the command that verifies the linting of code files.

Suggested fix for documentation

pre-commit run --from-ref=upstream/master --to-ref=HEAD --all-files

The command verifies the linting of code files, it looks for common mistake patterns
NOTE: An additional paragraph is preferred for this

☝ Is this issue description appropriate for the new issue?

@BobinMathew
Copy link
Contributor Author

take

@jreback jreback added this to the 1.3 milestone Jan 4, 2021
@jreback jreback merged commit 125441c into pandas-dev:master Jan 4, 2021
@jreback
Copy link
Contributor

jreback commented Jan 4, 2021

thanks @BobinMathew

@BobinMathew BobinMathew deleted the patch-1 branch January 4, 2021 04:37
@BobinMathew
Copy link
Contributor Author

Thankyou @MarcoGorelli and others for the support 🤝

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.

3 participants