Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Renamed "patch" to "pull request" #224

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

Open
bmispelon wants to merge 1 commit into django:main
base: main
Choose a base branch
Loading
from bmispelon:bye-bye-patch

Conversation

Copy link
Member

@bmispelon bmispelon commented Nov 6, 2024

Django hasn't used patches in earnest in probably
a decade, it's time to move on with the reality.

Django hasn't used patches in earnest in probably
a decade, it's time to move on with the reality.
Copy link
Member

I suppose we'll need to adjust all the contribution documentation as well though?

https://docs.djangoproject.com/en/5.1/search/?q=has%20patch

Copy link
Member Author

I suppose we'll need to adjust all the contribution documentation as well though?

https://docs.djangoproject.com/en/5.1/search/?q=has%20patch

Yes, definitely. I would like to see how things look like on my local version, then I'll file a PR for django/django. I'm putting the "on hold" tag on this for now.

(and thanks for you review by the way, I appreciate it ✨ )

charettes reacted with hooray emoji

Copy link
Member Author

For reference, the corresponding docs changes are tracked in https://code.djangoproject.com/ticket/35894#ticket

has_patch.order = 20
needs_better_patch = checkbox
needs_better_patch.label = Patch needs improvement
needs_better_patch.label = Pull request needs improvement
Copy link
Contributor

@sarahboyce sarahboyce Nov 7, 2024

Choose a reason for hiding this comment

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

Suggested change
needs_better_patch.label = Pull request needs improvement
needs_better_patch.label = Requested changes

maybe

Copy link

@nessita nessita left a comment

Choose a reason for hiding this comment

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

Thank you @bmispelon for starting this crusade!

I have commented on the ticket but I really think that we should not use a platform-dependent terminology (Pull Request is a GitHub specific term). I would suggest:

  • "Has proposal"
  • "Has fix"
  • "Has solution"

(I prefer the first one, since some proposal are not a fix nor a complete solution.)

And then: "Needs improvement" directly, considering that the other flags are "Needs docs" and "Needs tests".

Copy link
Member Author

Thanks for the feedback on this merge request! 🙃

I like your idea of "Needs improvement": it's shorter and matches the other two flags better.

As I wrote on Trac, I disagree your other point (and "proposal" seems too imprecise for me, plus it could be confused for the "P" of "DEP").

Copy link
Member

@bmispelon there are a lot of wording tweaks here that seem like they improve the clarity of the UI quite a bit. Should we look into getting those in? I see you labelled this "on hold", I assume the labels of the fields needs a lot of careful thought as the more stable the labels are the better. But for the "help text" type content of tickethacks.js, feels like we can always make more tweaks later?

Copy link
Member Author

The "on hold" label refers to the fact that this ticket depends on changes being accepted in the contributor documentation on django/django.

The changes I proposed there were met with conservatism that I found unwarranted, and I lost the appetite to contribute further as a result.

I don't think this ticket can move further until the discussion is resolved on the django/django issue, but that seems to be at a standstill right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahboyce sarahboyce sarahboyce left review comments

@nessita nessita nessita requested changes

+1 more reviewer

@charettes charettes charettes approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /