DOTD confirm less [ci skip] #16446
DOTD confirm less [ci skip] #16446
Conversation
bin/dotd
Outdated
| @@ -291,27 +291,20 @@ def do_dtt_subroutine | |||
| print_dtt_message | |||
| should_i 'DTT' do | |||
| view_commits base: 'test', head: 'staging' | |||
There was a problem hiding this comment.
Don't we want to view the commits before we declare whether we'll DTT or not?
There was a problem hiding this comment.
One can make that argument.
My feeling is that we're never actually going to change our decision based on what we see in the commits. Rather, I like to see the commits just to know the approximate complexity of what I have DTT'd.
Would be interested in knowing how others feel.
There was a problem hiding this comment.
Actually I'd do it in this order, I think:
view_commits base: 'test', head: 'staging'
print_dtt_message # Shorter than the commit list
should_i 'DTT' do
bin/dotd
Outdated
| DevelopersTopic.set_dtt 'yes' | ||
|
|
||
| new_sha = GitHub.sha 'test' | ||
| InfraTestTopic.set_green_commit new_sha |
There was a problem hiding this comment.
I might leave this Should I update test room status question - otherwise we don't have a natural flow for dealing with a red DTT. Could reword this question (or create a clearer prompt) for whether you're declaring the DTT green or red, but I would be surprised if continuing after "Wait for DTT () to complete, etc..." immediately marked the DTT green.
bin/dotd
Outdated
| DevelopersTopic.set_dtt 'yes' | ||
|
|
||
| new_sha = GitHub.sha 'test' | ||
| InfraTestTopic.set_green_commit new_sha |
There was a problem hiding this comment.
It seems a little aggressive to call the commit green just because I pressed enter after seeing wait_for "DTT (#{url}) to complete, re-run / investigate failures". How about replacing the aforementioned wait_for with should_i 'update test room status with green commit and timestamp'? That would leave you with the same number of prompts and hopefully a bit more clarity as to what is happening.
There was a problem hiding this comment.
to clarify a bit, you would DevelopersTopic.set_dtt 'yes' regardless of the answer, but you would only set_green_commit if the user said yes.
There was a problem hiding this comment.
Good points. I missed the purpose of this. Will update.
| end | ||
| DevelopersTopic.set_dtp 'yes' | ||
| sha = GitHub.sha 'production' | ||
| InfraProductionTopic.set_dtp_commit(sha) |
There was a problem hiding this comment.
For later: I wonder if updating the #infra-production topic should be part of our deploy process, rather than part of the DOTD script. It seems like something we rarely check and rarely (ever?) do manual intervention for.
Make our DOTD ask for you to confirm Y less often. In particular
(1) Make it so that it just updates the topic in developers without asking every time
(2) When asking if you want to DTT, merge to test immediately and then show the list of commits (instead of showing the list, and asking a second time if you want to merge).