Skip to content

Adding Stale label#531

Merged
willingc merged 4 commits into
python:masterfrom
eamanu:adding-stale-label
Sep 1, 2019
Merged

Adding Stale label#531
willingc merged 4 commits into
python:masterfrom
eamanu:adding-stale-label

Conversation

@eamanu

@eamanu eamanu commented Aug 28, 2019

Copy link
Copy Markdown
Contributor

In this PR I propose add the stale label on devguide.

Also, move OS-X label to above for a correct alphabetic order.

I would like document the awaiting * labels, but I don't know if in this section (Github label for PR) is the correct. Some opinion about it?

In this PR I propose add the stale label on devguide.

Also, move OS-X label to above for a correct alphabetic order.

@willingc willingc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sounds reasonable to me. @brettcannon or @Mariatta any objections?

Small typo with suggested change. Thanks @eamanu.

Comment thread triaging.rst Outdated
Commit @willingc suggestion

Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>
@brettcannon brettcannon self-requested a review August 28, 2019 19:21
Comment thread triaging.rst Outdated
@brettcannon

Copy link
Copy Markdown
Member

No objections to me, we just have to make sure to actually make the label if this gets merged. :)

Add @brettcannon suggestion

Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>
@eamanu

eamanu commented Aug 29, 2019

Copy link
Copy Markdown
Contributor Author

No objections to me, we just have to make sure to actually make the label if this gets merged. :)

Hi @brettcannon thanks for the review. I think the label is already created https://github.com/python/cpython/labels

@aeros aeros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the PR @eamanu! I recently added a section for the stale label to my PR (#530), but I removed it since it would probably be better to do it a separate PR (as you had done).

I think this is a great start, but I have some recommendations regarding the section. I figured it would be more readable to include it in the comments rather than trying to add a suggestion for each line.

Used for stale PRs or inactive for a long period of time. This label helps core developers identify quickly the PR candidate to be closed or make a ping to the author.

Grammar fixes:

Used for PRs that are stale or have been inactive for a long period of time. This label
helps core developers quickly identify PRs that are candidates for closure or require
a ping to the author.

Rather than using the word "stale" to describe the label, I think we should try to elaborate on what would be considered a stale PR. From my understanding, a PR could be considered stale if the changes are no longer relevant or if the author hasn't responded to feedback for a long period of time:

Used for PRs that include changes which are no longer relevant or when the author hasn't responded to feedback in a significant period of time. This label helps core developers to quickly identify PRs that are candidates for closure or require a ping to the author.

Lastly, we also should try to fit the maximum number of characters per line for formatting purposes. The maximum per line in reST is 80 (including indentation whitespace). Using the above suggestion, the lines would look like this:

stale
    Used for PRs that include changes which are no longer relevant or when the
    author hasn't responded to feedback in a long period of time. This label
    helps core developers quickly identify PRs that are candidates for closure 
    or require a ping to the author.

Tip: Using Python's builtin len() function in the interpreter is an easy way to measure the number of characters in each line. That's how I've been doing it recently. Writing a simple script for fitting as many words in each line within the 80 char limit would be better for large changes, but using len("text here") is quite convenient for small changes.

@eamanu

eamanu commented Aug 31, 2019

Copy link
Copy Markdown
Contributor Author

Hi @aeros167, thank you for the review.

Thanks for the PR @eamanu! I recently added a section for the stale label to my PR (#530), but I removed it since it would probably be better to do it a separate PR (as you had done).

I would like to document the awaiting-* labels. But I don't know if here is the correct sections. What do you think.

stale
    Used for PRs that include changes which are no longer relevant or when the
    author hasn't responded to feedback in a long period of time. This label
    helps core developers quickly identify PRs that are candidates for closure 
    or require a ping to the author.

I like how sound this, so I will add the suggestion. Thanks

@aeros

aeros commented Sep 1, 2019

Copy link
Copy Markdown
Contributor

@eamanu:

I would like to document the awaiting-* labels. But I don't know if here is the correct sections. What do you think.

We didn't include them in the initial iteration of the PR label section since those ones are automatically managed by the bots, but I don't think there would be an issue with adding them. That should probably be done in a separate PR after this one is finished though.

@willingc willingc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @eamanu 👍

@willingc willingc merged commit fd0d2fc into python:master Sep 1, 2019
AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Jun 17, 2022
* Adding Stale label

In this PR I propose add the stale label on devguide.

Also, move OS-X label to above for a correct alphabetic order.

* Update triaging.rst

Commit @willingc suggestion

Co-Authored-By: Carol Willing <carolcode@willingconsulting.com>

* Update triaging.rst

Add @brettcannon suggestion

Co-Authored-By: Brett Cannon <54418+brettcannon@users.noreply.github.com>

* Add @aeros167 suggestion
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.

5 participants