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

Rebind test on demand key - from Enter key #890

Closed
sreich opened this issue Jun 20, 2024 · 9 comments · Fixed by #897
Closed

Rebind test on demand key - from Enter key #890

sreich opened this issue Jun 20, 2024 · 9 comments · Fixed by #897

Comments

@sreich
Copy link

sreich commented Jun 20, 2024

Can we have a discussion on changing the keybinding for "tests on demand" Enter key? (I think this resides further upstream under ci-common rather than the gradle plugin...)

But from my perspective, Enter key seems about the worst key to choose to do such a thing. The interactive log prompt requires you to choose options and then press enter to select then. But then also the enter key by itself triggers the tests on demand...so people accidentally trigger it and then it gets in a whole loop of running a bunch of tests.

  • how about changing this default from Enter to "t" key for tests, or something else instead?
  • currently other menus are ? and q, the latter of which (if i'm not mistaken) requires Enter to be pressed

If not able to change the default/builtin (like if you have to/want to maintain compatibility), then barring that, can we at least rebind it with an option?

@cherylking
Copy link
Member

@sreich We have been discussing this amongst our team and internal users. There are definitely others that agree with you, but we would have to maintain compatibility. I will look into providing a way to customize the key binding for run tests on demand only (since it is different than the other actions that all require a letter key followed by Enter). When I have a snapshot for you to try, I will update here.

@cherylking
Copy link
Member

@sreich There is a 3.8.4-SNAPSHOT published in Sonatype now that includes a new config parameter to bind the "on demand tests" action to t plus Enter. Please give it a try and let me know if this solution will work for you (and if you have any feedback on the messages and help that are printed in dev mode related to this).

You need to add --changeOnDemandTestsAction to your libertyDev Gradle command to use this new functionality.

@sreich
Copy link
Author

sreich commented Aug 30, 2024

@cherylking all looks good to me! how can i add this to my config to be default? at the toolchain config level rather than a runtime command i have to enter each time

would using liberty.dev.setProperty give me that behavior i'm looking for?

@cherylking
Copy link
Member

@cherylking all looks good to me! how can i add this to my config to be default? at the toolchain config level rather than a runtime command i have to enter each time

would using liberty.dev.setProperty give me that behavior i'm looking for?

@sreich Unfortunately I don't think so. The libertyDev parameters are only specified on the command line. They were not implemented as properties. The libertyDevc parameters were implemented as properties. I'm not sure why the difference.

Let me see how difficult it would be to change this one parameter/property. So basically you want to be able to specify it in the build.gradle?

@sreich
Copy link
Author

sreich commented Aug 30, 2024

Okay. Yep, exactly, being able to set this in build.gradle is my ideal

@cherylking
Copy link
Member

@sreich I've updated the snapshot after adding changeOnDemandTestsAction to the dev extension as a configuration parameter. See the doc updates here.

@sreich
Copy link
Author

sreich commented Sep 3, 2024

@cherylking excellent, exactly what I was looking for. It works well :) Thanks Cheryl, I'm looking forward to the next release

@cherylking
Copy link
Member

@sreich Great. FYI...I have a developer working on the deprecation warnings now in preparation for Gradle 9. Would you prefer a release with this change before or after we have addressed issues #900 and #868 ?

@sreich
Copy link
Author

sreich commented Sep 3, 2024

@cherylking okay. whichever is most convenient for you, is fine by me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants