Unify paths #923
Unify paths #923
Conversation
jordisala1991
force-pushed
the
feature/unify_paths
branch
from
e1eb0fc
to
33d97b7
Compare
|
Ping @Elfet. Does this have any change to get merged? I have another PR depending on this one. |
|
This is cool release. But what about this?
Also what PR you want to?
Also i'm wondering is this functionality is really needed? I mean how many people will be using it? Because this PR bring a lot of config option which lead to complicate thinks and refactoring. For example releases_path and release_path differ only in one s letter which will lid to potential bugs. I think good naming will solve problems. |
| set('shared_dir', 'shared'); | ||
| set('dep_dir', '.dep'); | ||
|
|
||
| set('current_path', '{{deploy_path}}/{{current_dir}}'); |
There was a problem hiding this comment.
Now current_path will be returning symlink instead of real path with will lead to bugs. Also does not work with relative symlinks.
Why did you drop this code?
-set('current_path', function () {
- $link = run("readlink {{deploy_path}}/current")->toString();
- return substr($link, 0, 1) === '/' ? $link : get('deploy_path') . '/' . $link;
-});
There was a problem hiding this comment.
Why using a symlink will lead to bugs on the code? I am letting the OS to resolve the symlink instead of forcing a readlink.
| } | ||
|
|
||
| $link = run("readlink {{deploy_path}}/release")->toString(); | ||
| return substr($link, 0, 1) === '/' ? $link : get('deploy_path') . '/' . $link; |
There was a problem hiding this comment.
Same thing here. Why you drop this code? Now it's does not support relative symlinks.
There was a problem hiding this comment.
In this case, I wanted to change release_dirvariable to current_release like capistrano, but I wanted to mantain the PR with not too much changes.
| set('release_dir', 'release'); | ||
| set('releases_dir', 'releases'); | ||
| set('shared_dir', 'shared'); | ||
| set('dep_dir', '.dep'); |
There was a problem hiding this comment.
Do you really need to modify dir name of .dep?
There was a problem hiding this comment.
The reason to create variables for all directories is to be consistent in the code using those directories, like using a variable to avoid repetition of a constant value.
|
Please send links to capistrano same fature realization: gonna take closer look. |
|
I have another PR which focus on moving all Capistrano variables structure: https://www.admon.org/scripts/capistrano-variables-list/ |
I'm trying to unify all paths into a variables (similar to Capistrano). What do you think about this PR? I know it's not complete, and will require to change some things and some documentation maybe.
With this PR, one should be able to modify the structure of deployer using the new variables.
We can include new paths like:
current_releaseorprevious_releaseto simplify and make simple to userelease_pathoutside a release.