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

Make the stale bot ignore discussion urls #4436

Merged
merged 3 commits into from
Nov 10, 2021
Merged

Conversation

axic
Copy link
Member

@axic axic commented Nov 9, 2021

See https://github.com/actions/stale#exempt-issue-labels

After this change, if an issue is labeled discussion-url, it will be exempt from closure. We should reopen the actual discussion URLs which have been closed recently and label them too.

@eth-bot
Copy link
Collaborator

eth-bot commented Nov 9, 2021

EIP file name must be eip-###.md

@@ -27,8 +27,7 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
days-before-stale: 180
days-before-close: 14
stale-pr-label: 'stale'
exempt-issue-labels: 'discussion-url'
Copy link
Member Author

Choose a reason for hiding this comment

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

Any better suggestion for the label?

Copy link
Contributor

Choose a reason for hiding this comment

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

discussions-to maybe (so it matches the header field name in the EIP)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with that, though the value is an uri. Perhaps we should rename it to discussion-uri both label and in the EIPs? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would approve a PR that changes it to discussion-uri everywhere. If you are going to immediately follow this PR up with all of the necessary PRs to change to discussion-uri I'm fine with leaving this as is. If that is just an idea to put on the backburner, I think we should maintain consistency for now and have the label match the field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me know which way you would like to go, or if you would like to continue discussion, so I can know to merge or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to discussions-to here just to get this merged. We can make a bigger PR later and renaming the label is once off on Github, all tagged issues carry it over.

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Tentatively approve, but would like to hear a response to the stale-pr-label removal question before we merge.

@@ -27,8 +27,7 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
days-before-stale: 180
days-before-close: 14
stale-pr-label: 'stale'
exempt-issue-labels: 'discussion-url'
Copy link
Contributor

Choose a reason for hiding this comment

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

discussions-to maybe (so it matches the header field name in the EIP)?

@@ -27,8 +27,7 @@ jobs:
repo-token: ${{ secrets.GITHUB_TOKEN }}
days-before-stale: 180
days-before-close: 14
stale-pr-label: 'stale'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this removed as well? Feels unrelated to the addition of exempt-issue-labels?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have two different configurations here: one for issues the other for PRs.

This section here is for the issue, and has the appropriate stale-issue-label. The stale-pr-label is a useless setting in this context.

@MicahZoltu MicahZoltu merged commit 62db084 into master Nov 10, 2021
@MicahZoltu MicahZoltu deleted the stale-discussions branch November 10, 2021 14:38
PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
* Make the stale bot ignore discussion urls

* Fixes/cleanups

* Update stale.yml
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.

None yet

3 participants