mwr / magedeploy2

Magento2 Deployment with Deployer
MIT License
28 stars 10 forks source link

[FIX] Allow tag deployments #9

Closed cmuench closed 6 years ago

cmuench commented 6 years ago

At the moment only branches are processed. So the parameter of the robo task is not correct. This patch tries to fetch a list of tags from local already "fetched" repository. If the specified parameter matches a tag in the tag list, the tag method of the dep task is used instead of the branch method.

mwr commented 6 years ago

First of, to be sure to understand the nature of this change: Is this change targeting a functional error during the deployment or a cosmetic one?

The branch and / or tag that is provided to deployer is only used to create the directory name on the target server see \N98\Deployer\Service\GetReleasesNameService. And I think deploy:info does use the parameters branch / tag to print what is being deployed.

But other than that there is nor real use of these two paramters, since the whole update_code recipe is not in use during the push-deployment. Or did I miss something?

Second, none the less it would be great to differ between tags and branches. It is alreday being done during the update of the magento source code on the build server side in \Mwltr\MageDeploy2\Robo\RoboTasks::taskUpdateSourceCode 119:ff. In this method the detection of the tag is implemented as well and we should think about reusing this or at least re-using one of the "tag-detection" methods, thus not running the git commands twice.

I'd suggest to have the method getTagList detect the tag-list if not initialized and cache in a member variable. Then we could have a method isTag($tag) that used the getTagList and returns a boolean.

Regarding the implementation, I am not sure which git command works better in this scenario. Right now git show-ref --tags is used in method taskUpdateSourceCode. This pull request uses git tag -l and the only difference I can see is the information that is being printed.

cmuench commented 6 years ago

We used an old version MageDeploy which does not contain the tag fix. I close my PR.

mwr commented 6 years ago

Just for future reference: I used the idea and optimized the deployer integration by setting the branch option for branches and tag option for tags. This way deploy:info will print the correct output, though it is just a cosmetic change in the push deployment setup. See d06e4f866cd8096ed7b16c57affcc425763a65a7

And we still have to use exec in deployer 6 since the task results message does not contain the output, at least in my setup that is.