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

[Feature request] overriding the git hook so that we can make it run mix tasks inside of docker compose #82

Closed
oliverswitzer opened this issue Jun 30, 2021 · 6 comments · Fixed by #83

Comments

@oliverswitzer
Copy link

My team uses docker compose to run our Phoenix app and try to execute all of our mix tasks within that context with

docker-compose exec mix <insert task here>

(We do this to avoid having to set up the elixir/erlang ecosystem on every new dev machine)

It would be amazing if it were possible to override the default installed git hook to use docker-compose exec mix <elixir git hooks task> instead of mix <elixir git hook task> so we don't have to have mix on our local machine. I would add this functionality myself and open a PR, but I wasn't totally sure where I would do that... I know Mix.Tasks.GitHooks.Install would likely have to be modified, but I don't know how we would thread a config value that might override mix to be docker-compose exec mix... to that install function.

Ideally, we could specify this some key in our config for git hooks that would tell elixir_git_hooks to install the git hook with a slightly different prefix than mix. Does this seem possible?

Thanks in advance!

@jeffometer
Copy link

1 (ok, we're on the same team)

I see that the git hook template in priv needs to be changed, but my bash isn't good and I don't know how to change it there from a config in Elixir.

Also, we have a ./mix exacutable file that wraps the docker-compose command and can be used the same way mix is used. It might be easier to provide a "path to mix" that could work for people who don't have mix on their path and would also let us provide our ./mix script.

Thanks.

@qgadrian
Copy link
Owner

qgadrian commented Jul 3, 2021

@oliverswitzer @jeffometer guys could you please check #83 ?

I will take a deeper look later this weeking, but that should do it.

@qgadrian qgadrian pinned this issue Jul 3, 2021
@qgadrian qgadrian unpinned this issue Jul 3, 2021
@qgadrian qgadrian linked a pull request Jul 3, 2021 that will close this issue
@jeffometer
Copy link

That seems to work perfectly, thank you thank you!

@qgadrian
Copy link
Owner

qgadrian commented Jul 6, 2021

Awesome, I will make a couple of checks and submit a new release later today (UTC 2)

@jeffometer
Copy link

Just a note for me using a mix path of docker-compose exec web mix did not work (I got a TTY error), but this works docker exec --tty $(docker-compose ps -q web) mix. You may want to put that in your documentation if you think it is relevant to include.

@qgadrian
Copy link
Owner

qgadrian commented Jul 6, 2021

Just a note for me using a mix path of docker-compose exec web mix did not work (I got a TTY error), but this works docker exec --tty $(docker-compose ps -q web) mix. You may want to put that in your documentation if you think it is relevant to include.

Absolutely, I think makes sense to add this kind of information to avoid anyone running into the same problems. Thanks for the hint!

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

Successfully merging a pull request may close this issue.

3 participants