Remove remote management
yajo opened this issue · 5 comments
I've inspected a bit the core of the module and I think it is overly complicated in one regard: remote management. Let me explain.
On one hand, the module has the mode argument:
Lines 64 to 70 in d5d61f8
However, speaking in git, the mode is irrelevant. You can push and pull from local folder, remote ssh, remote https or even a local git bundle and git will abstract away all complexities. It just does its thing. So, the first thing that could be simplified is to just remove the mode and let git do its magic.
The next thing that is unnecessary IMHO is:
Lines 47 to 54 in d5d61f8
It is related to the previous point. If you want to use an authenticated https remote, you just need to pass that into the url:
git clone https://myuser:mytoken@github.com/owner/repo.git destination
If you just put the url as a possibly-secret argument, so that ansible won't report it when using -vvv, it should be enough.
Finally, this seems unneeded too:
Lines 105 to 109 in d5d61f8
Yes, we usually use remotes for managing git. However, in reality, you can just clone, pull and push by just using the git remote url directly. For example, this works perfectly fine:
git push https://github.com/lvrfrc87/git-acp-ansible development
... even if https://github.com/lvrfrc87/git-acp-ansible is not among that repo's remotes.
So, the module could forget about remotes, and just use the URL. FWIW that's what I'm doing in #53.
If you remove all this, the module will be much simpler and still quite useful. It will also not return a changed status just because the remotes are changed. Just some suggestions!
Hi @yajo,
Thanks a lot for raising this and the time spent on describing all the bits listed.
Let me try to explain my point and see if it make sense:
I am aware of all the git functionality you described and I remember asking myself years ago if that could be a right way or not to develop the module. However, I believed that having all the arguments specified can facilitate even less experienced users to figure out how to use the module - a.k.a git.
Having mode: https arguments with assertion for token to be present, drive the user to properly insert the right argument.
I remember having people asking me on slack why were getting some errors due to different origin used - which they were not aware of it. Having origin argument, push the inexperience user to research around the functionality of such argument.
However, your point is valid and I believe that, if well documented - with git doc references for example - could be a valid solution. I would be interested in seeing a PR and think to a version 2.0. My only concern would be backward compatibility with previous version.
FYI - I am planning to move git-acp under separate library and use ansible as wrapper around it. Not sure how long it will take me but I will be happy to have you onboard: https://github.com/lvrfrc87/git-acp
Nice! However for the time being I'm moving away from this project. I like it and I think it's necessary, but for my use case it ended up being an overkill because these issues described here made it impossible for me to make it work properly, whereas a simple ansible.builtin.shell invocation with a 5-lines bash script works fine. So although I appreciate your offer, I'm sorry I'll have to reject it for now.
It'd be nice if the project improves these aspects and finally I'm able to use it, because of course it's better to use an ansible module than a bash script! I hope you understand...
Thanks for your help on this. The changes proposed are now in version 2.0 release