Skip to content

Commit

Permalink
Add autoload to composer.json (#67)
Browse files Browse the repository at this point in the history
  • Loading branch information
sirbrillig committed Jul 23, 2018
1 parent 328000e commit 40642df
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@
"scripts": {
"test": "./vendor/bin/phpunit"
},
"autoload": {
"psr-4": {
"NeutronStandard\\": "NeutronStandard/"
}
},
"require": {
"php": "^7.0"
"php": ">=7.0"

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jul 23, 2018

Contributor

This is bit dangerous, since it promises weak contract that can be easily broken.
Why is that desirable?

This comment has been minimized.

Copy link
@sirbrillig

sirbrillig Jul 23, 2018

Author Member

🤔 I guess this might be a bit too wide a range.

I was following the composer guide, which suggests this format of php version requirement, and I was also thinking that maybe we would want this to be able to be used in future versions of php without needing to update the required version. However, I suppose that a major version bump would imply backwards breaking changes, which could break this code. Reverted in #68

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jul 23, 2018

Contributor

I was following the composer guide, which suggests this format of php version requirement, and I was also thinking that maybe we would want this to be able to be used in future versions of php without needing to update the required version

Could you link that please? It shall be fixed.
In 99 % of these cases in open-source projects I see safe limits like "symfony/symfony": "^4.0"

However, I suppose that a major version bump would imply backwards breaking changes, which could break this code.

Exactly 👍

Thanks

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jul 23, 2018

Contributor

Btw, I see there are lot of duplicated sniffs that are already in PHP CS Fixer. Often 1:1 feature.
Do you have some personal reasons for that?

This comment has been minimized.

Copy link
@sirbrillig

sirbrillig Jul 23, 2018

Author Member

This was where I was looking: https://getcomposer.org/doc/01-basic-usage.md#platform-packages

Btw, I see there are lot of duplicated sniffs that are already in PHP CS Fixer.

From what I understand, phpcs and php-cs-fixer are not compatible, so this provides sniffs for the phpcs engine. Our code base is already using phpcs and adding php-cs-fixer would be a much bigger change than adding new sniffs for the existing engine.

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jul 23, 2018

Contributor

Thank you 👍 , I've already send a PR :)


Our code base is already using phpcs and adding php-cs-fixer would be a much bigger change than adding new sniffs for the existing engine.

Well, they're different tools, but there is a way :) instead of writing duplicated rules, you could write new ones making your rock even more!

Could you link some repository using this package (smaller the better)? I'd like to take a look if that's possible for your case.

This comment has been minimized.

Copy link
@sirbrillig

sirbrillig Jul 23, 2018

Author Member

Unfortunately I think the only repos that I am using this for at the moment are private 😓 The code is quite old, and these sniffs are intended to help clean it up.

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Jul 23, 2018

Contributor

I see. If you ever get to use PHP 7.1, you can look at EasyCodingStandard. Tool I made to combine PHP_CodeSniffer and PHP CS Fixer in nice UX way.

To skip all the boring stuff, jump right to How to Migrate From PHP_CodeSniffer to EasyCodingStandard in 7 Steps, that describes the basics.

This comment has been minimized.

Copy link
@sirbrillig

sirbrillig Jul 23, 2018

Author Member

Thanks!

},
"require-dev": {
"sirbrillig/phpcs-variable-analysis": "^2.0.1",
Expand Down

1 comment on commit 40642df

@TomasVotruba
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, I was about to add this 👍 missing autoloading

Please sign in to comment.