Repository navigation
Single Command Installation of Rector + Targets for Managing Tools - #20
Conversation
Adds an installation target that appears when `tools/rector` doesn't exist Adds `rector` and `rector-fix` targets when the rector directory is found Adds PHP code for export-ignoring things in .gitattributes Signed-off-by: George Steel <george@net-glue.co.uk>
…tially rector) and fix conditionals. Conditionals that test for file/dir existence with `$(shell ...)` should be stipped with `$(strip $(shell ...))` instead of using `echo -n` Signed-off-by: George Steel <george@net-glue.co.uk>
Signed-off-by: George Steel <george@net-glue.co.uk>
…ect PHP version to use Signed-off-by: George Steel <george@net-glue.co.uk>
weierophinney
left a comment
There was a problem hiding this comment.
Just a quick question: a number of the targets and conditionals create paths by appending a directory captured in a variable; can you be certain that these directory variables end in a trailing slash? As written, it looks like there is potential for them to omit it, which would lead to unexpected paths...
| @@ -0,0 +1,35 @@ | |||
| RECTOR_DIRECTORY ?= $(PROJECT_DIR)tools/rector | |||
There was a problem hiding this comment.
is $(PROJECT_DIR) guaranteed to end with a /?
There was a problem hiding this comment.
In local testing, $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) yields directories with trailing slashes, so I've adopted that convention for these types of variables, which is where most of them come from.
|
|
||
| _do-install-rector: | ||
| @$(call MK_INFO,"Installing Rector") | ||
| @$(DOCKER_RUN) ${DOCKER_IMAGE_NAME} php $(CURRENT_DIRECTORY)migrate $(PROJECT_DIR) |
There was a problem hiding this comment.
will $(CURRENT_DIRECTORY) always have a trailing /?
| @@ -0,0 +1,36 @@ | |||
| CURRENT_DIRECTORY := $(dir $(abspath $(lastword $(MAKEFILE_LIST)))) | |||
| RECTOR_DIRECTORY := $(PROJECT_DIR)tools/rector | |||
There was a problem hiding this comment.
Same comment as for Rector.mk file - can we guarantee $(PROJECT_DIR) has a trailing slash?
There was a problem hiding this comment.
Also, it seems like the trailing slash is a documented "standard":
https://www.gnu.org/software/make/manual/html_node/File-Name-Functions.html#index-dir
Adds an installation target
install-rectorthat appears whentools/rectordoesn't existAdds
rectorandrector-fixtargets when the rector directory is foundAdds PHP code for export-ignoring things in .gitattributes
Adds
update-toolsandbump-toolstargets that collect all targets for updating and bumping these tool versions.