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

[StandaloneLineConstructorParamFixer] Ignore constructor without arguments #32

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Oct 31, 2023

When you have a constructor without arguments, the fixer changes it to:

-protected function __construct() {
+protected function __construct(
+) {

Not sure how to solve this. Any advice?

When you have a constructor without arguments, the fixer changes it to:

```diff
-protected function __construct() {
+protected function __construct(
+) {
```

Not sure how to solve this.
@TomasVotruba
Copy link
Member

I'd skip the ) as following token or get arguments count and skip if === 0.

1 similar comment
@TomasVotruba
Copy link
Member

I'd skip the ) as following token or get arguments count and skip if === 0.

@ruudk
Copy link
Contributor Author

ruudk commented Oct 31, 2023

While running this test locally, I notice that my changes in src/Fixer/Spacing/StandaloneLineConstructorParamFixer.php are not used.

Upon running it with Xdebug, I notice that the tests use this file:

vendor/symplify/easy-coding-standard/vendor/symplify/coding-standard/src/Fixer/Spacing/StandaloneLineConstructorParamFixer.php

Is there a developer contributing guideline that I'm missing?

I checked out the repo, ran composer install, and ran the tests. I expected it to use the code in src but apparently it uses the scoped vendor code in vendor/symplify/easy-coding-standard/vendor/.

@ruudk
Copy link
Contributor Author

ruudk commented Oct 31, 2023

@TomasVotruba I think I found a way to solve it. Could you approve the workflow?

@ruudk ruudk changed the title Add failing test for StandaloneLineConstructorParamFixer [StandaloneLineConstructorParamFixer ] Ignore constructor without arguments Oct 31, 2023
@ruudk ruudk changed the title [StandaloneLineConstructorParamFixer ] Ignore constructor without arguments [StandaloneLineConstructorParamFixer] Ignore constructor without arguments Oct 31, 2023
@ruudk
Copy link
Contributor Author

ruudk commented Oct 31, 2023

@TomasVotruba Please approve again. Why is there approval needed?

@TomasVotruba
Copy link
Member

That's Github default for first-contributors. Do you know how to turn it off?

@TomasVotruba
Copy link
Member

Thanks for the fix 👍

@TomasVotruba TomasVotruba merged commit aceaedf into symplify:main Oct 31, 2023
8 checks passed
@ruudk ruudk deleted the failing-test branch October 31, 2023 18:56
@ruudk
Copy link
Contributor Author

ruudk commented Oct 31, 2023

Thanks for the hint and the merge. Please tag it 💙

@TomasVotruba
Copy link
Member

Expect next week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants