Skip to content

Single Command Installation of Rector + Targets for Managing Tools - #20

Merged
gsteel merged 4 commits into
laminas:1.0.xfrom
gsteel:rector
Oct 7, 2026
Merged

gsteel merged 4 commits into
laminas:1.0.xfrom
gsteel:rector

Conversation

@gsteel

@gsteel gsteel commented Oct 7, 2026

Copy link
Copy Markdown
Member

Adds an installation target install-rector 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

Adds update-tools and bump-tools targets that collect all targets for updating and bumping these tool versions.

gsteel added 2 commits October 7, 2026 11:48
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>
@gsteel gsteel added this to the 1.0.0 milestone Oct 7, 2026
@gsteel
gsteel requested a review from a team October 7, 2026 11:23
Signed-off-by: George Steel <george@net-glue.co.uk>
Comment thread migrations/InstallStandaloneRector/templates/rector.php Outdated
Comment thread migrations/InstallStandaloneRector/templates/rector.php Outdated
…ect PHP version to use

Signed-off-by: George Steel <george@net-glue.co.uk>

@froschdesign froschdesign left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍🏻

Comment thread migrations/InstallStandaloneRector/templates/rector.php
@gsteel gsteel self-assigned this Oct 7, 2026
@gsteel
gsteel merged commit c513571 into laminas:1.0.x Oct 7, 2026
17 checks passed
@gsteel
gsteel deleted the rector branch October 7, 2026 12:06

@weierophinney weierophinney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Comment thread makefiles/Rector.mk
@@ -0,0 +1,35 @@
RECTOR_DIRECTORY ?= $(PROJECT_DIR)tools/rector

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is $(PROJECT_DIR) guaranteed to end with a /?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will $(CURRENT_DIRECTORY) always have a trailing /?

@@ -0,0 +1,36 @@
CURRENT_DIRECTORY := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
RECTOR_DIRECTORY := $(PROJECT_DIR)tools/rector

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as for Rector.mk file - can we guarantee $(PROJECT_DIR) has a trailing slash?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants