Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Remove invalid tool call from composer check script #2600

Merged
merged 1 commit into from
Oct 12, 2024

Conversation

sbuerk
Copy link
Contributor

@sbuerk sbuerk commented Apr 2, 2024

With commit [1] development dependencies have been removed
from the composer.json, but not from the composer.lock.

This indicates a inproper use of composer and should be
taken care in review of pull-requests. A good practice here
is to etablish a rule that composer commands needs to be
added to the commit message and the pull-request.

Other removed dependency and downgraded dependencies have
been fixed or readded meanwhile, still missing the phpcs
tool for the check composer script.

The CONTRIBUTION.md states to execute composer check
before creating a pull-request, which is literally broken
due to the missing dependency.

This change removes the php sniffer call from the check
script. Additionally, the composer scripts are enhanced
to use the same php binary used to invoke composer itself
to mitigate issues using the wrong php version on systems
with multiple php version binaries.

Further consideration should be to use the check command in
the Github Action workflow to have a border if development
toolchain is gonna be broken with a change.

@sbuerk sbuerk force-pushed the stefan-4 branch 2 times, most recently from 5c01957 to 1dab8ee Compare April 2, 2024 13:16
@sbuerk sbuerk changed the title [BUGFIX] Ensure all required development dependencies [BUGFIX] Remove invalid tool call from composer check script Apr 2, 2024
@coveralls
Copy link

coveralls commented Apr 2, 2024

Coverage Status

coverage: 96.918%. remained the same
when pulling 5df53c2 on sbuerk:stefan-4
into 43785fe on PHPOffice:master.

@Progi1984 Progi1984 added this to the 1.3.0 milestone Aug 23, 2024
@Progi1984 Progi1984 modified the milestones: 1.3.0, 1.3.1 Aug 30, 2024
With commit [1] development dependencies have been removed
from the `composer.json`, but not from the `composer.lock`.

This indicates a inproper use of `composer` and should be
taken care in review of pull-requests. A good practice here
is to etablish a rule that composer commands needs to be
added to the commit message and the pull-request.

Other removed dependency and downgraded dependencies have
been fixed or readded meanwhile, still missing the `phpcs`
tool for the `check composer script`.

The `CONTRIBUTION.md` states to execute `composer check`
before creating a pull-request, which is literally broken
due to the missing dependency.

This change removes the php sniffer call from the check
script. Additionally, the composer scripts are enhanced
to use the same php binary used to invoke composer itself
to mitigate issues using the wrong php version on systems
with multiple php version binaries.

Further consideration should be to use the check command in
the Github Action workflow to have a border if development
toolchain is gonna be broken with a change.

[1] PHPOffice@c3e34a0
@Progi1984 Progi1984 merged commit feadceb into PHPOffice:master Oct 12, 2024
13 checks passed
@Progi1984
Copy link
Member

Thanks @sbuerk for your contribution

@sbuerk sbuerk deleted the stefan-4 branch October 12, 2024 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants