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

feat(options): add new options to avoid stale based on comments #508

Closed
wants to merge 15 commits into from

Conversation

C0ZEN
Copy link
Contributor

@C0ZEN C0ZEN commented Jun 15, 2021

Changes

  • Add 3 new options to avoid staling when a comment is added
  • The scope of remove-stale-when-updated will be reduced, resulting in a breaking change (@luketomlinson for the release notes!) when using the option at false

⚠️ this is a breaking change

Context

Helping to close (maybe) #441, #470, #435?
Closes #390 due to no activity

@C0ZEN C0ZEN force-pushed the feature/remove-stale-when-commented branch from 09d1712 to 07423be Compare June 18, 2021 08:18
@C0ZEN
Copy link
Contributor Author

C0ZEN commented Jun 22, 2021

I'm kind of stuck with the issue it caused.
May need help...

@jeremystretch
Copy link

@C0ZEN what's the issue?

@C0ZEN
Copy link
Contributor Author

C0ZEN commented Jun 23, 2021

It makes sense to have this feature but the current implementation is actually helping the case where an issue is marked as stale then still in the same workflow process it for closing process. In such case the freshly staled issue update date is set as today and with the current implementation provided in this PR it will consider the issue as no longer stale if a comment was added. If the new option to install when commented is disabled there is no problem

@luketomlinson luketomlinson mentioned this pull request Jun 24, 2021
2 tasks
@C0ZEN C0ZEN force-pushed the feature/remove-stale-when-commented branch from 473ca4b to 4755004 Compare June 24, 2021 21:06
src/classes/issues-processor.ts Show resolved Hide resolved
__tests__/main.spec.ts Outdated Show resolved Hide resolved
@C0ZEN C0ZEN force-pushed the feature/remove-stale-when-commented branch from a8ac4e8 to a8f503a Compare July 18, 2021 14:11
@C0ZEN
Copy link
Contributor Author

C0ZEN commented Jul 18, 2021

The last added test is actually failing because just after staling, the workflow continue and then consider the issue as stale but to unstale due to shouldRemoveStaleWhenUpdated && issueHasUpdate being both true.

@C0ZEN
Copy link
Contributor Author

C0ZEN commented Jul 18, 2021

I close it because I just don't understand the whole workflow.
Each time I change something a test is failing, and actually, I can't define if its bad tests or a bad changes.
The fact that the specs are mocking the label creation date and all the logic to calculate if either or not it should stale or close seems terrible IMO.
What could be nice is to rewrite this whole part — code and specs included — and I just don't have the courage to do so.
Well, free to continue the work from this branch or just tackle this from scratch.

@C0ZEN C0ZEN closed this Jul 18, 2021
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.

None yet

3 participants