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

[Bugfix] Remove stale when updated without comment #717

Merged
merged 1 commit into from Jun 20, 2022

Conversation

0x5c
Copy link
Contributor

@0x5c 0x5c commented Apr 15, 2022

The existing logic for remove-stale-when-updated only considers presence of comments, ignoring the presence of any other kind of update.

This fixes it so it follows the documented behaviour for that setting.

There is no effect from this change on tests (all pass). I suspect that the tests simply do not cover the case of updates, only comments; the new logic is equivalent to the old one for the case of new comments.
I would have included new tests to add to the coverage, but I have no idea how to achieve that as this is my first time ever touching TypeScript code.

Changes

  • The check for removing stale now takes into account the value of issueHasUpdate

Context

Fixes #715

@0x5c
Copy link
Contributor Author

0x5c commented May 8, 2022

Rebased & retested

@0x5c 0x5c changed the title Remove stale when updated without comment [Bugfix] Remove stale when updated without comment May 9, 2022
@0x5c
Copy link
Contributor Author

0x5c commented Jun 8, 2022

Only by looking at #748 that I found out that npm run pack has to be run and committed, as that step is not mentioned in CONTRIBUTING.md.

That needed step has now been done (and tests got run again, for good measure).

@luketomlinson
Copy link
Collaborator

Apologies for the delay @0x5c, I'm working through backlog of PRs.

Thanks for adding this fix. I also updated contributing guidelines per your comment. I'll approve once conflicts are resolved.

The existing logic for `remove-stale-when-updated` only considers presence of
comments, ignoring the value of `issueHasUpdate`.

This commits fixes it so it follows the documented behaviour for that setting.

Fixes actions#715
@luketomlinson luketomlinson merged commit 29e800e into actions:main Jun 20, 2022
5 checks passed
@0x5c 0x5c deleted the removewhenupdated branch Jun 20, 2022
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.

remove-stale-when-updated not honoured in unmarking logic
2 participants