slashdeploy icon indicating copy to clipboard operation
slashdeploy copied to clipboard

Confusing message when autodeploy blocked by my own lock

Open turadg opened this issue 8 years ago • 3 comments

screen shot 2017-06-27 at 6 47 34 pm

It's good that the autodeploy is prevented by a lock, but this message is confusing. Maybe if it just said I was going to autodeploy… ?

turadg avatar Jun 28 '17 01:06 turadg

if i locked production and then merged to master, i would actually hope that autodeploy would be locked down. even if it was me who did the merge. i think the expected workflow for this given the current behavior would be to manually /deploy after getting the notification.

because i have the expectation that a lock would block even my own merge from triggering autodeploy, the idea of changing this behavior scares me. i can see one might expect either behavior: locking allowing only the locker's mergers, or not. for a delicate operation like deployment, i would personally prefer to stay on the conservative end.

was the issue that you didn't see the slack notification about prod being locked until too late, blocking an important deploy? maybe there's another solution to the issue you had?

1.actually print on the github pull request page that if you merge the PR, it won't be autodeployed. idk how this could be possible. perhaps via commit status checks. 2. add a button to the "but it's currently locked" slack notification to override the lock.

sumeet avatar Jun 28 '17 10:06 sumeet

Agree with @sumeet. I think it'd be dangerous to let auto deploys go through, even if you hold the lock. The current behavior isn't an accident.

With that said, the messaging could definitely be improved, and we could also include a message button to force the deployment through.

I really like the idea of adding commit statuses for locks. It was brought up in https://github.com/remind101/slashdeploy/issues/62, but I haven't really got around to implementing it.

ejholmes avatar Jun 28 '17 10:06 ejholmes

Good point @sumeet. I agree so I'll revise title to make this a copy change.

turadg avatar Jun 28 '17 15:06 turadg