Issue 1264 bugfix refactor #1276
Conversation
…ugins, add forwarded template for AshStartScriptPlugin
|
Hi @archeuclid, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
There was a problem hiding this comment.
Thanks @archeuclid for putting so much effort and thought into this PR 🤗
As mentioned in a comment below I really want to understand why we need to add new settings for ash and not just reuse the bash settings. I know that the name is not ideal (shell would probably be better), but adding these settings adds a lot of complexity and maintenance burden.
| @@ -69,9 +69,11 @@ import sbt._ | |||
| * enablePlugins(AshScriptPlugin) | |||
| * }}} | |||
| */ | |||
| @deprecated("Use AshStartScriptPlugin", "1.4.1") | |||
There was a problem hiding this comment.
I wonder why we need to do this. As mentioned in #1264 (comment) I'm currently not seeing the benefit as we copy all settings, which are identical, but add the complexity of having to deal with versions of a script where actually only one should exist.
There was a problem hiding this comment.
I just wanted to rename so it is ashSTARTSCRIPTplugin not just ashSCRIPTplugin. I agree we can reuse keys, will change that.
| @@ -1,105 +1,155 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
I really like the cleaned up version of this scripts. This is by far more readable and predictable
|
Hello Ok This project need lots of refactor. The scripts must be part of "staging", since it is so close related to JavaAppPackaging. I suggest to write specs for scripts then incorporate them in the "stage". Also, suggest changing the name from "stage" ("Stager" etc). It is a DevOps term and must be delineated from SoftwareDevs. Sorry, guys. Lots of stuff to work on hand right now. Use the template .sh in any way you want. I will delete this pull request next time I am here or reject it. Cheers |
|
Is there any way to use what's here? |
|
@briantopping unfortunately not. We have no releases for pull requests. You will need to checkout this pull request and build on your own :( Or you finish this PR and we release :) |
|
No worries, I converted the code that depends on this to pass arguments as environment variables. My shell skills are poor at best. |
You are in good company @briantopping Since kubernetes we have to fight more with YAML than anything else 😂 |
Hello
Added AshStartScriptPlugin with associated keys, templates, tests. Made it so it does not depend on BashStartScriptPlugin keys and defines. It has to be explicitly enabled with
enablePlugins.The generated scripts do not have quite the same
usageas bash/bat scripts. Maybe someone can realign them later and add to docs.