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

Renamed "patch" to "pull request" #224

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bmispelon
Copy link
Member

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

@bmispelon
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 ✨ )

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

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

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

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

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.

5 participants