Skip to content

Conversation

ryanwilsonperkin
Copy link
Contributor

@ryanwilsonperkin ryanwilsonperkin commented Sep 11, 2019

Fixes #17
cc @skjnldsv as requester: Does this match what you were thinking?

Breaking change: This would switch the parameters from:

  • exempt-pr-label -> exempt-pr-labels
  • exempt-issue-label -> exempt-issue-labels

By switching to a list syntax, we can still support a single "exempt" label (eg. at my company we just have a label called "pinned" that we would use to avoid being marked stale) but can now also support many different "exempt" labels.

Update (13/04/2020):
Because we can't use arrays as arguments to the action, this PR has been switched to use a comma-delimited string to indicate multiple labels.


if (exemptLabel && isLabeled(issue, exemptLabel)) {
if (exemptLabels.some(exemptLabel => isLabeled(issue, exemptLabel))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@skjnldsv
Copy link

Awesome! Yes this is what I have in mind :)

exempt-issue-labels:
  - with team
  - with boss
  - lunch
  - with client
{
	'exempt-issue-labels': [
		'with team',
		'with boss',
		'lunch',
		'with client'
	]
}

@ryanwilsonperkin
Copy link
Contributor Author

Hey @damccorm thanks again for your help o #11
This PR proposes a breaking change to the interface added in that one, does your team have any opinion on breaking changes in this project?

@Gustry
Copy link

Gustry commented Oct 3, 2019

Oh yes please, we need this option!
Thanks for the patch.

@LekoArts
Copy link

LekoArts commented Oct 29, 2019

You won't be able to write it like this as an end-user:

exempt-issue-labels:
  - with team
  - with boss
  - lunch
  - with client

Because GitHub doesn't accept that as a valid format.

You can use YAML and the | pipe though like:

exempt-labels: |
  not stale
  important

Then you need to parse the incoming labels:

exemptLabels: parseLabels(core.getInput("exempt-labels"))

To get an array of strings:

// Since GitHub doesn't allow arrays in workflow files
// You need to use YAML and | pipe
// Therefore this function splits on new lines and creates an array
function parseLabels(files: string): string[] {
  return files.split(/\r?\n/).reduce<string[]>(
    (acc, line) =>
      acc
        .concat(line.split(","))
        .filter(pat => pat)
        .map(pat => pat.trim()),
    []
  )
}

I used that in our fork: gatsbyjs#1

@ryanwilsonperkin
Copy link
Contributor Author

@LekoArts thanks for the heads up, I wasn't aware of that limitation. In that case though, I'd probably default to using comma-separated values for the options instead of newline-separated with a pipe operator. I suspect that if we use the pipe operator to put it on multiple lines, users might infer that it's an array and prefix each line with a - which would result in the labels not being applied properly.

So I'd now imagine the default value for this to be the empty string, and the example will give users would be:

exempt-issue-labels: foo,bar,baz

Since this PR has remained un-reviewed for a little while, I'll wait until I've heard back from the owners about whether they'd like to accept this functionality before making the change to the PR.

@qJake
Copy link

qJake commented Nov 16, 2019

Looking forward to this in the next release!

I don't think there will be an issue since the actions are versioned, e.g. I reference this action like this:

uses: actions/stale@v1

So as long as this is included in a "v2" for example, it shouldn't break anything existing.

@goanpeca
Copy link

goanpeca commented Dec 9, 2019

In that case though, I'd probably default to using comma-separated

Yes, this seems like a good option since a comma is not a valid character in a label name and would make the parsing much simpler 👍

@dmodoomsirius
Copy link

I do hope this becomes a thing soon!!!

@qJake
Copy link

qJake commented Jan 6, 2020

Eagerly awaiting this in actions/stale@v2 (or v1.1, or whichever!).

@tbeswick96
Copy link

Any eta for this?

@JamesSingleton
Copy link

@goanpeca,

Any update on when this would potentially get merged and released?

@goanpeca
Copy link

Not my call 🤷‍♂

@JamesSingleton
Copy link

@chrispat maybe you can shed some light on this?

@ces131
Copy link

ces131 commented Apr 8, 2020

Really want to add this to a few repos but can't until multiple exempt labels are added - any updates??

@LekoArts
Copy link

LekoArts commented Apr 9, 2020

As mentioned by me in another PR already: Feel free to use our (Gatsby) fork:
https://github.com/gatsbyjs/stale

@Gustry
Copy link

Gustry commented Apr 9, 2020

As mentioned by me in another PR already: Feel free to use our (Gatsby) fork:
https://github.com/gatsbyjs/stale

Thanks, I will give it a try!

It's somehow sad that GitHub (which owns this actions repository) doesn't look at PR on their own platforms.

@hross
Copy link
Contributor

hross commented Apr 13, 2020

This repo is getting pretty stale and I'm taking it over to clean it up and get some of these suggestions merged. I like this one @ryanwilsonperkin if you want to add the comma parsing we can get it merged and pushed with the next release (soon).

And yep @Gustry it's sad this repo got neglected for so long 😢 (but we will rehab it shortly).

@hross hross self-assigned this Apr 13, 2020
@ryanwilsonperkin
Copy link
Contributor Author

Hey @hross thanks for taking over! I'll push an update to switch this over to comma-parsing instead.

@qJake
Copy link

qJake commented Apr 13, 2020

@ryanwilsonperkin Thanks for the quick turnaround!

@hross Thanks for taking this over - really hoping to see a release very soon with this functionality in it! ❤️

@hross
Copy link
Contributor

hross commented Apr 15, 2020

this looks good if you want to resolve the conflicts I'll merge it.

@ryanwilsonperkin
Copy link
Contributor Author

On it

@ryanwilsonperkin
Copy link
Contributor Author

ryanwilsonperkin commented Apr 15, 2020

@hross ready 🎉

@hross I notice there's now a dist/index.js file that seems to be the compiled output from TypeScript. Is there some transpile step that I have to run on this PR in order to update that file?

@hross
Copy link
Contributor

hross commented Apr 15, 2020

@ryanwilsonperkin yes you need to run npm run all (runs linting) or npm run pack to generate the new dist file.

@hross
Copy link
Contributor

hross commented Apr 15, 2020

(I'll update the readme with better instructions)

@JamesSingleton
Copy link

@hross, can't wait for this to go in!

@hross hross merged commit 6127f8e into actions:master Apr 16, 2020
@ryanwilsonperkin ryanwilsonperkin deleted the multiple-exempt-label branch April 16, 2020 01:38
richardlau added a commit to nodejs/diagnostics that referenced this pull request Nov 2, 2022
Version 2 of actions/stale changed the option for
exempt issue labels from the singular 
`exempt-issue-label` 
to the plural 
`exempt-issue-labels`.

Refs: actions/stale#19
Refs: nodejs/build#2704
RafaelGSS pushed a commit to nodejs/diagnostics that referenced this pull request Nov 2, 2022
Version 2 of actions/stale changed the option for
exempt issue labels from the singular 
`exempt-issue-label` 
to the plural 
`exempt-issue-labels`.

Refs: actions/stale#19
Refs: nodejs/build#2704
richardlau added a commit to nodejs/node-addon-api that referenced this pull request Nov 2, 2022
From `actions/stale@v2` the option for exempt labels
was renamed to `exempt-issue-labels` (plural). Also
it appears that this repository is using `never-stale`
(with a hyphen instead of a space character).

Refs: actions/stale#19
legendecas pushed a commit to nodejs/node-addon-api that referenced this pull request Nov 14, 2022
From `actions/stale@v2` the option for exempt labels
was renamed to `exempt-issue-labels` (plural). Also
it appears that this repository is using `never-stale`
(with a hyphen instead of a space character).

Refs: actions/stale#19
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
From `actions/stale@v2` the option for exempt labels
was renamed to `exempt-issue-labels` (plural). Also
it appears that this repository is using `never-stale`
(with a hyphen instead of a space character).

Refs: actions/stale#19
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.

Support multiple exempt labels