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

Option stale-issue-message required? #521

Closed
jasongrout opened this issue Jul 6, 2021 · 7 comments · Fixed by #522
Closed

Option stale-issue-message required? #521

jasongrout opened this issue Jul 6, 2021 · 7 comments · Fixed by #522
Labels
bug Something isn't working

Comments

@jasongrout
Copy link

jasongrout commented Jul 6, 2021

Describe your issue

We have omitted the stale-issue-message option (in fact, we still have skip-stale-issue-message set, but that seems ignored now). According to the docs at https://github.com/actions/stale#stale-issue-message:

You can skip the comment sending by omitting the option or by passing an empty string.

So it seems since we have omitted the stale-issue-message parameter, we should be fine - issues should be marked stale without a comment. However, in our logs, we see https://github.com/jupyterlab/jupyterlab/runs/2994705101?check_suite_focus=true#step:2:3995 :

[#5869] Skipping this issue because it should be marked as stale based on the option days-before-stale (​https://github.com/actions/stale#days-before-stale​) (30) but the option stale-issue-message (​https://github.com/actions/stale#stale-issue-message​) is not set

and the relevant issue is not marked stale. Is giving a stale-issue-message mandatory now, with an empty string signifying no message?

Your stale action configuration

https://github.com/jupyterlab/jupyterlab/blob/cd7ac23f738c9a8792bbb76a256e84a82535446c/.github/workflows/answered.yml

name: 'Close answered issues'
on:
  schedule:
    - cron: '30 1 * * *'

jobs:
  stale:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/stale@v3
        with:
          skip-stale-issue-message: true
          days-before-stale: 30
          days-before-close: 7
          stale-issue-label: 'status:Closing as Answered'
          only-issue-labels: 'status:Answered'

Further context

@jasongrout
Copy link
Author

Oh, I think I'm getting confused between unreleased master, and what is necessary in v3. It seems in v3 I need to give both the skip-stale-issue-message and a blank message? But in v4 I won't need to specify either to get no comment?

@C0ZEN
Copy link
Contributor

C0ZEN commented Jul 7, 2021

@jasongrout indeed, the default branch is main which provide the actual state of the action including unreleased features.
If you use the v3, you need to combine this option with the skip-stale-issue-message (you can check the doc from the v3 tag https://github.com/actions/stale/tree/v3).

For the next major (v4) or the current main, if the option is empty, no message will be sent.

const skipMessage = issue.isPullRequest
? this.options.stalePrMessage.length === 0
: this.options.staleIssueMessage.length === 0;

However, since there is a default value

stale/action.yml

Lines 9 to 11 in 678bfc7

stale-issue-message:
description: 'The message to post on the issue when tagging it. If none provided, will not mark issues stale.'
required: false

I think that the documentation should not mention that omitting will not send a message.
I can provide an update of the doc for this.

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jul 7, 2021
…ll not send a message

To be sure, what would be even better is to add a test using the default config (because the main issue is that the default options of the specs are not matching the ones from the action).

Closes actions#521
@jasongrout
Copy link
Author

Thanks!

@jasongrout
Copy link
Author

Adding stale-issue-message: '' still didn't let skipping work for us (same error), but according to DynamoDS/Dynamo@8ac994c#diff-8162bb3b3d4da55f30fcbb603b5d630b21b397777a6a42eff0e0fba30bc746a1R15, it looks like stale-issue-message needs to be set to a nonempty value for the skipping to work in v3?

@C0ZEN
Copy link
Contributor

C0ZEN commented Jul 9, 2021

@jasongrout sorry it's hard to tell.
What do you think about using the main branch until the v4 is released?
Don't know why it's taking so much time, and it's obviously not a good practice but mostly no new feature or breaking change will be added until the v4 is out I suppose, so it's OK ish.
WDYT?

@C0ZEN
Copy link
Contributor

C0ZEN commented Jul 9, 2021

The PR is ready.
Nice catch :)

@jasongrout
Copy link
Author

No problem. For now I did a stale-issue-message of '.', like in the link I posted. Thanks for fixing things in v3, we really appreciate how this action helps us manage answered issues!

C0ZEN added a commit to C0ZEN/stale that referenced this issue Jul 15, 2021
…ll not send a message

To be sure, what would be even better is to add a test using the default config (because the main issue is that the default options of the specs are not matching the ones from the action).

Closes actions#521
C0ZEN added a commit to C0ZEN/stale that referenced this issue Sep 19, 2021
…ll not send a message

To be sure, what would be even better is to add a test using the default config (because the main issue is that the default options of the specs are not matching the ones from the action).

Closes actions#521
C0ZEN added a commit to C0ZEN/stale that referenced this issue Sep 21, 2021
…ll not send a message

To be sure, what would be even better is to add a test using the default config (because the main issue is that the default options of the specs are not matching the ones from the action).

Closes actions#521
luketomlinson added a commit that referenced this issue Sep 22, 2021
…e option will not send a message (#522)

* chore(assignees): add logs

* docs(stale-issue-comment): update the docs to remove that omitting will not send a message

To be sure, what would be even better is to add a test using the default config (because the main issue is that the default options of the specs are not matching the ones from the action).

Closes #521

* test(comment): add more coverage to test the stale issue message

* docs(readme): improve the wording

Co-authored-by: Luke Tomlinson <luketomlinson@github.com>

* refactor: simplify the code to use the stats for the specs

* chore(rebase): fix rebase issue

* chore(statistics): fix issue due to rebase

Co-authored-by: Luke Tomlinson <luketomlinson@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants