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

Automatic version detection #1098

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

uuf6429
Copy link
Contributor

@uuf6429 uuf6429 commented Aug 24, 2024

This pull request adds a new ruleset, PhpAuto, that picks the right ruleset based on the minimum required PHP version defined in composer.json of the current working directory.

New dependencies added:

  1. ext-json (backword compatible, since it was already required by php-cs-fixer)
  2. composer/semver (supported since PHP 5.3, so it should also be fine)

Additional Changes:

  • project itself now uses the new ruleset (I figured it would be less maintenance + eating your own dog food)
  • updated the readme file with the same reasoning; most people would want to automate this

Design Notes:

  • at first I was going for a PHP version -> ruleset class (e.g. [53 => Php53::class]) map but then I thought it would be less work to just add an interface and build the class name dynamically

@uuf6429
Copy link
Contributor Author

uuf6429 commented Aug 25, 2024

I've been thinking, this feature I'm proposing could also be implemented as a separate repo/package.

If that approach is preferred, I would gladly consent to having it under the same vendor.

@uuf6429

This comment was marked as resolved.

Copy link

codecov bot commented Aug 28, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 36 lines in your changes missing coverage. Please review.

Project coverage is 99.68%. Comparing base (b2b47fe) to head (62761f0).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/RuleSet/PhpAuto.php 0.00% 32 Missing ⚠️
src/PhpVersion.php 60.00% 4 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##                main    #1098      +/-   ##
=============================================
- Coverage     100.00%   99.68%   -0.32%     
- Complexity        70       84      +14     
=============================================
  Files             22       23       +1     
  Lines          11310    11352      +42     
=============================================
+ Hits           11310    11316       +6     
- Misses             0       36      +36     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@localheinz localheinz self-assigned this Sep 2, 2024
@localheinz
Copy link
Member

localheinz commented Nov 24, 2024

Thank you for your pull request, @uuf6429!

This project solves to issues for me:

  • distributing configurations across projects
  • consciously committing to a (or the minimum supported) PHP version in a project and using the corresponding configuration

Why would you want to automatically select a different configuration depending on the PHP version you are currently running in?

@uuf6429
Copy link
Contributor Author

uuf6429 commented Nov 24, 2024

Why would you want to automatically select a different configuration depending on the PHP version you are currently running in?

If I had multiple projects in different PHP versions, it would be nice to define a custom configuration that applies to all projects without worrying that some particular setting will break in a specific version.

For example, say I have a custom config to enable short-array-syntax (PHP 5.4+) and space-around-concatenation, I wouldn't want to manually check for the PHP version if I happen to use it on PHP 5.3.

Anyway, this feature wasn't taken favourably at work...apparently we're fine reinventing our own version of a wheel, so feel free to close this PR if you see no value.

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.

3 participants