-
Notifications
You must be signed in to change notification settings - Fork 9
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
Interactive Prompt Reminder for lanes waiting for input/confirm #302
Conversation
5d6ac13
to
3919652
Compare
lib/fastlane/plugin/wpmreleasetoolkit/helper/interactive_prompt_reminder.rb
Outdated
Show resolved
Hide resolved
f927d2c
to
5c9b79d
Compare
say "An interactive prompt is waiting for you in the Terminal!"
5c9b79d
to
a281b87
Compare
@wordpress-mobile/owl-team This is now ready for review. (As a bonus, if we merge #299 soon, then that PR would make an easy candidate for a small new version to test the gem push by Buildkite CI 😛 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I first saw the PR and understood it involved monkey patching, my first reaction was "woo woo 🤚 monkey patching is dangerous! is the added 'risk' worth the reminders?" Then, I forgot about the prompt for 3 builds in a row and wished I had a spoken reminder there. 😅
I tested it and it worked as described in terms of the configuration behavior but my terminal (iTerm 2 Build 3.4.9beta1) did not beep nor bounce. I suspect this has to do with my notifications configurations in Big Sur, though 🤔
The icon did show the notification badge with a "1" when I went back and checked 👍
lib/fastlane/plugin/wpmreleasetoolkit/helper/interactive_prompt_reminder.rb
Outdated
Show resolved
Hide resolved
module FastlaneCore | ||
# NOTE: FastlaneCore::UI delegates to the FastlaneCore::Shell implementation when output is the terminal | ||
class Shell | ||
DEFAULT_PROMPT_REMINDER_MESSAGE = 'An interactive prompt is waiting for you in the Terminal!'.freeze |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity: do you hear any difference when using the exclamation mark? I tried `say "potato"; say "potato!"; say "potato. potato!" and they all sounded the same 😄 🤷♂️
Since the code calls |
h/t @mokagio Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>
@mokagio I found this article about iTerm vs Terminal beep/bell settings. So I think the reason why you didn't have a beep in your iTerm is just because you probably have "Silence bell" option enabled in your iTerm preferences. 🙃 |
I've merged |
What
This introduces a monkey-patch on fastlane's interactive UI methods like the
UI.input
andUI.confirm
which will remind you (via beep + badge and icon jumping in Dock, and optionally an opt-in spoken alert) that the Terminal is waiting for an answer from you if you didn't provide one after some time.Why
This is particularly very useful if, like me, your often run a command like
bundle exec fastlane new_beta_release
then switch another app to work on something else, thinking that you can work on something else while the beta is building on CI… only to realize 10mn or even 30mn or more later that the CI never started, and that you've wasted all that waiting time for nothing, because you forgot that the lane was waiting for you to confirm the action first.With this change, if you go do other things and forget that the prompt is waiting for you, you'll now be reminded about it ✨
Demo
(Turn sound on, as I narrate and have enabled the
FASTLANE_PROMPT_REMINDER_MESSAGE
option too)interactive_prompt_reminder-720p.mov
Customization
This code uses Monkey-Patching 🐒 to redefine fastlane's
UI.…
interactive methods (UI.input
,UI,confirm
, etc) and wrap their original implementations around a helper method (UI.with_reminder
) which handles the delay+alerting logic.By default, those
UI.…
methods will be automatically patched to generate a system beep and make your Terminal icon jump after 30s then 3mn then 10mn. You can opt-out of the auto-patching usingFASTLANE_PROMPT_REMINDER_DISABLE_AUTO_PATCH=1
env var.When you don't opt-out of the auto-patching (the default), you can also refine the behavior of those reminders:
FASTLANE_PROMPT_REMINDER_DELAYS
env varsay
command – using theFASTLANE_PROMPT_REMINDER_MESSAGE
env var.default
,yes
,true
or1
to use the default message, i.e "An interactive prompt is waiting for you in the Terminal!"To test
0. Point your Pluginfile to this PR
develop
branch in your client repo (e.g. WPiOS)fastlane/Pluginfile
to this branch, runbundle update fastlane-plugin-wpmreleasetoolkit
to update theGemfile.lock
, and commit the changes (because we need a clean repo in order for thenew_beta_release
lane not to fail)1. Test the default behavior
bundle exec fastlane new_beta_release
in your terminal, then switch to another app and/or go drink a coffee (without waiting or replying to the prompt)n
to the prompt to cancel the new betagit checkout
back to your test branch (asnew_beta_release
would have switched to the release branch during its exececution)2. Test a custom delay + default spoken message
FASTLANE_PROMPT_REMINDER_DELAYS=3,5 FASTLANE_PROMPT_REMINDER_MESSAGE=default bundle exec fastlane new_beta_release
n
to the prompt, thengit checkout
back to your test branch3. Test a custom spoken message
FASTLANE_PROMPT_REMINDER_DELAYS=3,5 FASTLANE_PROMPT_REMINDER_MESSAGE="Hello?" bundle exec fastlane new_beta_release
n
to the prompt, thengit checkout
back to your test branch4. Test opting out completely
FASTLANE_PROMPT_REMINDER_DISABLE_AUTO_PATCH=1 FASTLANE_PROMPT_REMINDER_DELAYS=3,5 FASTLANE_PROMPT_REMINDER_MESSAGE=default bundle exec fastlane new_beta_release
n
to the prompt, thengit checkout
back to your test branchMergeability
We maybe want to wait for us to do the
2.0.0
major release of therelease-toolkit
first before merging this and ship it as part of a2.1.0
, just because if we need to fix or revert things introduced by the breaking changes from #300 due to some bug, it would make more sense to keep the major release focused on the big change only. This is why I've kept this PR as Draft for now, and why I didn't add theCHANGELOG
entry just yet.That being said, if it's approved in time, and you feel it has its place in the upcoming
2.0.0
, I don't have strong feelings about it being included sooner.