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

Feature: Exempt labels from action #11

Merged
merged 8 commits into from
Aug 27, 2019

Conversation

ryanwilsonperkin
Copy link
Contributor

Fixes #10

Hey! This is the first action that we've tried out on our repos and it seems great. I'm an engineer on our Developer Systems team at Wave and we support features like this to help developers get their work done.

We had an internal request for this feature, and then I saw that it was already listed as an issue so I decided to take a crack at it. Thanks to @vincentschen, @christopheranderson, and @damccorm for the discussion that's happened so far on #10.

I'm not sure how best to test a change like this, and I'm open to any advice from the maintainers/owners of this project. Thanks

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

This looks awesome - thanks for the contribution! Left a few comments, but it generally looks great.

I'm not sure how best to test a change like this, and I'm open to any advice from the maintainers/owners of this project.

So far, we've mostly just been doing integration testing, so basically just run the action for the use cases surrounding the change. With that said, I want that to change and have an open issue (#12) to add units. For now, doing some integration testing is probably fine though - this seems like a pretty safe change to me.

src/main.ts Outdated Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
src/main.ts Show resolved Hide resolved
Since the exempt label parameter is not required, we don't want to both
checking for it if it was not defined.
This variable just tracks the number of operations we're using to avoid
rate limiting. Since the exempt check wouldn't result in a rate-limited
request, we don't need to reduce the variable.
action.yml Outdated Show resolved Hide resolved
@damccorm
Copy link
Contributor

Looks great, thanks for the contribution!

@damccorm damccorm merged commit 9f69ef1 into actions:master Aug 27, 2019
@damccorm
Copy link
Contributor

Published! Both actions/stale@v1 and actions/stale@v1.1.0 should have this

@ryanwilsonperkin ryanwilsonperkin deleted the exempt-labels branch August 27, 2019 20:12
@ryanwilsonperkin
Copy link
Contributor Author

Lovely! Thanks for your help @damccorm

@NARKOZ
Copy link

NARKOZ commented Sep 5, 2019

Is it possible to set several exempt labels?

@ryanwilsonperkin
Copy link
Contributor Author

@NARKOZ no

@skjnldsv
Copy link

Is it possible to set several exempt labels?

@NARKOZ no

created issue #17 :)

@nchammas
Copy link

nchammas commented Dec 9, 2019

@damccorm

Published! Both actions/stale@v1 and actions/stale@v1.1.0 should have this

The latest version shows as 1.0.0. Is 1.1.0 somehow not fully released yet?

@damccorm
Copy link
Contributor

damccorm commented Dec 9, 2019

@bryanmacfarlane to take a look - the tag is definitely valid - https://github.com/actions/stale/releases, but it isn't showing up that way in the marketplace

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.

Exempt specific labels from action?
5 participants