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

adding a change #4562

Closed

Conversation

Student-ShivamChauhan
Copy link

@Student-ShivamChauhan Student-ShivamChauhan commented Jan 28, 2025

Description

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@Student-ShivamChauhan
Copy link
Author

review changes and tell me right or wrong, this is my first contribution so please merge it

@MeGaGiGaGon
Copy link
Collaborator

MeGaGiGaGon commented Jan 28, 2025

Thanks for the contribution, I have a few concerns that need addressed before this is ready for merging:

  1. Make sure you have read this page https://black.readthedocs.io/en/latest/contributing/the_basics.html . A lot of the feedback I will give already exists on this page. This includes how to run the tests and how to run black on itself.
  2. It looks like you ran some sort of tool that changed comments # to strings ", and single line triple quoted doc strings """ to single quotes ". Please revert this, as it is not equivalent. You should disable whatever other formatting tools you are using while working on Black, since part of the CI process is to run Black through itself, which will fail with the changes you've made.
  3. As the checklist says, you need a changelog entry. It doesn't have to be anything long or complex, but please add one. Again, see the above link.
  4. Please add test cases so you can be sure your changes work. I two examples in # fmt: skip doesn't work with multiline strings #4513 that can be used as test cases.
  5. Add Fixes #4513 to your opening PR description, so that it is added to the close on merge list.
  6. Please edit the name of this PR to be more descriptive
  7. I'm not sure if you did, but please do not use AI when making PRs for Black.

@MeGaGiGaGon
Copy link
Collaborator

On further review (and asking with other project members), this is being closed for not meeting the minimum effort requirements. Thank you for your time, but everyone else (including me) is also donating time to help this project, so we expect a minimum amount of effort to be put into PRs. If you are still interested in helping, see my above comment for some parts of a quality PR. Thank you.

@Student-ShivamChauhan
Copy link
Author

i understand what you tell (i run the black repository then pass so request to merge it ), but please give me hints and write how to improve this repository with my good efforts, this is my first contribution so please guide me i know about python but i do my first effort for black repository and i am interested in it also do work with you, sorry for my previous work i try again my best so give me some task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants