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

Add Probot lock configuration #17041

Merged
merged 7 commits into from Mar 29, 2018

Conversation

Projects
None yet
5 participants
@lee-dohm
Member

lee-dohm commented Mar 27, 2018

Description of the Change

Add lock configuration in preparation for adding Lock Threads to the Atom organization.

Alternate Designs

N/A

Why Should This Be In Core?

It needs to be everywhere.

Benefits

The triage team will have to deal less with people saying "me too" on closed issues that, while they may appear related, often aren't.

Possible Drawbacks

People may get upset that we're preventing them from commenting on old issues. Of course, they'll have to open new issues to let us know about it, which is the whole point.

Applicable Issues

Too many to list.

/cc @Ben3eeE @50Wliu because you mentioned that you have a canned response for these kinds of things and maybe that would be better than what I've tossed together for the lock comment here.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Mar 27, 2018

Our canned response is like

if you can reproduce the issue after starting Atom in safe mode atom --safe then please open a new issue and completely fill out the provided issue template.

I like mentioning safe mode in the canned response in addition to it being a prerequisite in the issue template since it double confirms that safe mode has been checked.

Something like this is how I would write it if I combined the two and changed it up some

This issue is now locked since it had enough time for everyone to give further feedback on after it was closed. If you can still reproduce this issue in safe mode atom --safe then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented Mar 27, 2018

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Mar 27, 2018

@lee-dohm Sure thing. I updated it in 5ad7a0a

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented Mar 27, 2018

daysUntilLock: 180
# Comment to post before locking. Set to `false` to disable
lockComment: >
This issue is now locked since it had enough time for everyone to give further

This comment has been minimized.

@Arcanemagus

Arcanemagus Mar 27, 2018

Contributor

This issue is now locked since it had enough time for everyone to give further feedback on after it was closed.

I think at the least this should have the "after it was closed" part removed, since the timeout is based on inactivity, not the close date.

This issue is now locked as it has had enough time for everyone to give further feedback on it.

Or alternatively, with more finality to it:

This issue is now locked as the time for any further feedback has passed.

This comment has been minimized.

@rsese

rsese Mar 27, 2018

Member

I like the the conciseness of the last form 👍

This comment has been minimized.

@Ben3eeE

Ben3eeE Mar 28, 2018

Member

The bot only locks closed issues. I wanted to have the keyword closed in there to communicate that only closed issues are locked. But I agree that the current wording might be misleading. I don't get any immediate ideas on how to change this to include both things 💭

Clearly communicating how and why the bot acts is something I learned is important when rewording the stale message after feedback. If we remove closed there is someone that will believe we started locking all issues and PRs.

This comment has been minimized.

@Ben3eeE

Ben3eeE Mar 28, 2018

Member

Maybe

This issue has been automatically locked since there has not been any recent activity after it was closed.

This comment has been minimized.

@Arcanemagus

Arcanemagus Mar 28, 2018

Contributor

The bot only locks closed issues. I wanted to have the keyword closed in there to communicate that only closed issues are locked.

This message would only be shown on closed issues, but making it explicit doesn't hurt, especially since this would likely be the last comment on these threads.

Your new message works for me!

@rsese

This comment has been minimized.

Member

rsese commented Mar 27, 2018

I'm assuming the bot will lock all closed issues currently? Not sure if it can be configured to not lock some types of issues? help-wanted issues for example, I'm guessing we wouldn't want to lock those?

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Mar 27, 2018

Not sure if it can be configured to not lock some types of issues?

It looks like currently this isn't possible.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Mar 28, 2018

@rsese Great question about help-wanted issues. There are also issues that are closed without being resolved. For example due to inactivity by stale.

Options for these issues that I can see are:

  1. Contribute to the bot to add an exempt labels configuration option
  2. Lock them and reword the message to include actions that we want users to take if they are tackling a closed+locked issue that is not resolved.

So something like this for the 2nd option (Quick draft):

Thanks for your contribution!

This issue has been automatically locked since there has not been any recent activity after it was closed.

  • If you are still experiencing this issue when running atom in safe mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue.
  • If this issue was closed without being resolved and you want to contribute with a PR to fix this issue then please reach out to us on Slack to discuss the issue and changes required to Atom.

This doesn't really make sense as a message for something like merged PRs? 💭

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Mar 28, 2018

Atom should be capitalized in the safe mode link 😉.

Would it make sense to just submit a PR there adding the ability to filter out specific labels?

This doesn't really make sense as a message for something like merged PRs?

This is currently only applying to issues.

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented Mar 28, 2018

Would it make sense to just submit a PR there adding the ability to filter out specific labels?

Yes, I think it would 👍

Ben3eeE added some commits Mar 29, 2018

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Mar 29, 2018

With dessant/lock-threads@8a29d89 being released in version 0.2.0 of the lock threads bot we can now use exemptLabels. I added this in 090ff41

@lee-dohm This should be ready for review now. Do you need to update the bot version we use as well?

@dessant

This comment has been minimized.

dessant commented Mar 29, 2018

This is currently only applying to issues.

only: issues needs to be uncommented to limit locking to issues.

This doesn't really make sense as a message for something like merged PRs?

Yeah, it would be better to have a different comment for PRs, I'll copy the way Stale handles configuration in the near future and update you here, for now please limit the bot to issues only.

@dessant

This comment has been minimized.

dessant commented Mar 29, 2018

@Ben3eeE, regarding the bot instance, the latest version has been deployed to Glitch, only the config needs to be added after the app has been installed.

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented Mar 29, 2018

Do you need to update the bot version we use as well?

I haven't installed the bot yet. I didn't want to install it until the configuration was in place.

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented Mar 29, 2018

@dessant thank you so much for helping out!!

@dessant

This comment has been minimized.

dessant commented Mar 29, 2018

@Ben3eeE, no problem, thank you for Atom. 😍

@lee-dohm lee-dohm merged commit a3e5fc0 into master Mar 29, 2018

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lee-dohm lee-dohm deleted the add-lock-configuration branch Mar 29, 2018

@dessant

This comment has been minimized.

dessant commented Mar 29, 2018

Just noticed the number of inactive issues you have: https://github.com/atom/atom/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aclosed+updated%3A%3C2017-09-30

This will mean 30 emails every hour in the next two weeks for repo watchers, 10k in total. Then there are the issue subscribers. 😨

Alternatively you can opt to lock issues which have been inactive for more than a year without posting a comment, then switch back to the current config two weeks from now. That way repo watchers would get 1.3k emails instead of 10k.

I agree that locking without mentioning any reason could be confusing for those who stumble upon these threads, even if you decide to do so for very old issues only. I'm just not sure how will people react to their inboxes.

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented Mar 29, 2018

We had a similar flood back when stale was first enabled 😆.

@dessant

This comment has been minimized.

dessant commented May 13, 2018

Setting custom config options for pull requests is now possible: https://github.com/dessant/lock-threads#configuration.

Though pull requests stop being processed after a while regardless of the config values due to a search API issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment