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

Set a default value for $isDebug in SymfonycastsSassExtension #21

Merged
merged 2 commits into from
Oct 23, 2023

Conversation

evertharmeling
Copy link
Contributor

At the moment when you run php bin/console config:dump symfonycasts_sass it throws an exception.

In SymfonycastsSassExtension.php line 74:

Typed property Symfonycasts\SassBundle\DependencyInjection\SymfonycastsSassExtension::$isDebug must not be accessed before initialization

Setting a default value fixes this.

Copy link
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

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

Seems reasonable 👍

@WebMamba
Copy link
Contributor

WebMamba commented Oct 17, 2023

I just don't get why you have your config kernel.debug set to null? Are you using the bundle outside a Symfony App? Or do you use a custom Kernel?

@bocharsky-bw
Copy link
Member

@WebMamba Hm, I can reproduce this issue on the Symfony Demo app, the first time I run symfony console config:dump symfonycasts_sass - it works, but the further runs it throws

Typed property Symfonycasts\SassBundle\DependencyInjection\SymfonycastsSassExtension::$isDebug must not be accessed before initialization

until I clear the cache. Then it repeats: works 1st time and does not the next times

@evertharmeling
Copy link
Contributor Author

evertharmeling commented Oct 20, 2023

Yes, like bocharsky-bw describes.

However, that would probably mean that the 'real' value is lost after cache is warmed. And with the first commit resulting in $isDebug always being false...

I've updated the PR of an other approach...

@WebMamba
Copy link
Contributor

Yes, good catch! Looks good to me

Copy link
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Good simplification :)

@bocharsky-bw bocharsky-bw merged commit 542b3c9 into SymfonyCasts:main Oct 23, 2023
10 checks passed
@evertharmeling evertharmeling deleted the patch-1 branch November 2, 2023 15:30
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

3 participants