-
Notifications
You must be signed in to change notification settings - Fork 353
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
Stale label not being removed after new activity #441
Comments
This line looks like it explains the output that I'm seeing: stale/src/classes/issues-processor.ts Lines 483 to 484 in a61d498
To me, this means that stale/src/classes/issues-processor.ts Lines 452 to 457 in a61d498
|
One more thing to add, a colleague pointed out to me that the label is removed as expected during a scheduled run (we have it run every 24 hours). It seems that the times where it doesn't work are on |
@Rylan12 I am thinking about using |
I haven't seen a change since I've opened this issue, so I'd say don't bother with it at the moment. There are several downsides to having this (seemingly) useless But if you try and it works for you I'd love to see the configuration to try to figure out if I'm just doing something wrong 😅 I'm probably going to remove the |
I think, this PR can fix this problem with |
This would be great to get this merged! |
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
Thanks for working on this, @C0ZEN! ✨🎉 Did a quick test with this and it looks like it will fix our issue. Once this is released and we start using it I'll follow up if things still aren't working. |
@Rylan12 sure, thanks for the feedbacks and I hope it's working as expected. |
Hi @C0ZEN The issue was already old enough for being stale but the workflow didnt start yet. After a comment on issue https://github.com/MAGICCC/teststale/issues/3 the workflow triggered and added a comment on https://github.com/MAGICCC/teststale/issues/4 + https://github.com/MAGICCC/teststale/issues/5 and added stale label + removed it again.
Maybe I did something wrong... |
@MAGICCC there is bad condition on the logger on the PR I provided sadly 🐛 stale/src/classes/issues-processor.ts Lines 618 to 626 in f2ae27a
FYI, I do not test the logs 😞 So, the correct logs should be:
Nice catch! Regarding the strange behaviour, I think I can sum up like this:
|
const updated_at = new Date().toString();
const daysBeforeClose = 2;
function updatedSince(timestamp, num_days) {
const daysInMillis = 1000 * 60 * 60 * 24 * num_days;
const millisSinceLastUpdated =
new Date().getTime() - new Date(timestamp).getTime();
return millisSinceLastUpdated <= daysInMillis;
}
updatedSince(updated_at, daysBeforeClose) // true; This is currently the behaviour. |
@Rylan12 "I hope it's working as expected."... The only time I say this and bingo a bug is introduced... |
This one should be reopened then, due to the revert. |
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
@luketomlinson could you reopen please? |
Seems same as #509 |
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
Are there any suggested workarounds for this? We were running into this as well (here is an issue where the comment is ignored: projen/projen#723 and here is how the GitHub action was set up: https://github.com/projen/projen/blob/139d9e3fb66dffdcc976c5a6e873c98099258fcf/.github/workflows/stale.yml ) |
Thanks, @luketomlinson and @C0ZEN! Did a quick test using the |
Indeed, tested it with my testrepo and its look fine! Thanks you. I am looking forward for v4 now :) |
@luketomlinson well done |
Helping to close actions#441, actions#470, actions#435? Closes actions#390 due to no activity BREAKING CHANGES: the options related to remove-stale-when-updated will only check the updates, not the comment. It is only impactint the configurations using the value at false
not fixed. Just commented selectize/selectize.js#1824 and stale label not removed |
Describe your issue
The
stale
label isn't being removed from PRs when there's new activity even thoughremove-stale-when-updated
isn't set.Your stale action configuration
Further context
I made a comment on Homebrew/brew#11140 (comment) that should have caused the action to remove the
stale
label. The action did run successfully (the logs are available here). Here are the relevant lines from the logs:The line that says
Pull request marked stale on: 2021-05-08T15:44:35Z
seems to be incorrect, as the stale label was added 16 hours ago before that time. I wonder if the action is using the time of the latest comment as the time that the stale label was added.The text was updated successfully, but these errors were encountered: