Add /deploy command for GitHub deployment #9
Conversation
lib/hermetic/slash/deploy.ex
Outdated
| if String.contains?(repo, "@") do | ||
| String.split(repo, "@") | ||
| else | ||
| [repo, "production"] |
There was a problem hiding this comment.
This should be an OTP param.
lib/hermetic/slash/deploy.ex
Outdated
|
|
||
| env = | ||
| case command do | ||
| [] -> "production" |
| @@ -0,0 +1,41 @@ | |||
| defmodule Hermetic.Github do | |||
There was a problem hiding this comment.
Inconsistent casing (Github here, but not in PR title or on this site).
There was a problem hiding this comment.
tbh I really hate typing GitHub, I'd rather change it all to Github.
| # Finish string or raise if unclosed | ||
| defp do_split(<<>>, "", acc, false), do: Enum.reverse(acc) | ||
| defp do_split(<<>>, buf, acc, false), do: Enum.reverse([buf | acc]) | ||
| defp do_split(<<>>, _, _, _), do: raise("quoted string not closed") |
There was a problem hiding this comment.
Is manual splitting here justified over OptionParser.split/1?
There was a problem hiding this comment.
Note that this code was only moved. It is almost the same as the source of OptionParser.split/1. I don't know if we need to support ' in the command right now, but I think it's better to keep parsing consistent with the other command.
|
re separation of concerns: I think it would be unwieldy to run separate bots for every small feature. The best solution is probably to create a single slack bot with plugins for each of these features. That would take me a significant amount of effort though, so I don't think it should be covered in this PR. Most of the code here can still be used verbatim for plugins, so no effort is lost. If you want to do this refactoring, feel free, otherwise I might take it up in the future when I have a better understanding of Elixir. |
|
re separation of concerns: We never heard back from you about this, even though I prompted you multiple times. Making the bot modular is out of scope for this PR. |
| @@ -3,6 +3,10 @@ use Mix.Config | |||
| config :hermetic, Hermetic, | |||
| cowboy_options: [port: 59468] | |||
|
|
|||
| config :hermetic, Hermetic.Slash.Deploy, | |||
| default_ref: "master", | |||
| default_env: "production" | |||
There was a problem hiding this comment.
While I'm aware that this is configurable, I'd prefer the default to be staging, so that a deployment to production needs to be explicitly specified.
There was a problem hiding this comment.
I chose production because this is what the github api defaults to, so I think it makes more sense because that is what people expect a bot to do that just wraps the github deployments api.
We've had this conversation multiple times over several months, even back to when it was Vito's job, and nothing has changed since then. This repo is for a generic, user-configurable YouTrack -> Slack bridge, not some Serokell bot.
This feature is out of scope for this program. |
No description provided.