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

Make command lazily loaded #373

Merged
merged 1 commit into from
Nov 9, 2020

Conversation

kefzce
Copy link
Contributor

@kefzce kefzce commented Sep 21, 2020

Hi there.
Since Symfony 3.4^ allows us to provide console command name for compile-time registration all console command should be registered by this way, if not
every time when someone run ./bin/console Symfony should compile all container to fetch command name and set parameter manually

Also here is a way to check if on your Symfony project some commands register not properly

bin/console debug:container --parameter=console.command.ids

Example output from my project now

  Parameter             Value                                                            
 --------------------- ----------------------------------------------------------------------------- ------------ 
  console.command.ids   ["console.command.public_alias.Sentry\SentryBundle\Command\SentryTestCommand
 --------------------- ------------------------------------------------------------------------------------------

we are using
"sentry/sentry-symfony": "^3.0" // 3.5.2
This is the simple fix which not breaking BC because BC layer already on \Symfony\Component\Console\Command\Command

Also here some ref where the same problem was fixed by this way

btw, I'm not sure about the target branch, should be 3.5 instead?

Copy link

@za4me za4me left a comment

Choose a reason for hiding this comment

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

LGTM!

@Jean85 Jean85 changed the base branch from master to 3.5.x September 21, 2020 14:01
Copy link
Collaborator

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Change LGTM 👍 but I had to change the target branch, so you should drop the additional commits from master.

Please also add a changelog entry too.

@kefzce
Copy link
Contributor Author

kefzce commented Sep 21, 2020

Change LGTM 👍 but I had to change the target branch, so you should drop the additional commits from master.

Please also add a changelog entry too.

Sure, thanks ill rebase pr a bit later from the target branch.

@kefzce kefzce force-pushed the feature/lazy-load-commands branch 3 times, most recently from 11471c0 to 5b2ef9c Compare September 21, 2020 16:35
@kefzce kefzce requested a review from Jean85 September 21, 2020 18:35
@ste93cry
Copy link
Collaborator

ste93cry commented Nov 7, 2020

Can you please rebase once more and fix the broken build (it should be enough to change the regex of the error message of PHPStan so that instead of hardcoding the version number uses a match pattern)? Also, in the CHANGELOG can you please add a reference to this PR?

@kefzce
Copy link
Contributor Author

kefzce commented Nov 7, 2020

Can you please rebase once more and fix the broken build (it should be enough to change the regex of the error message of PHPStan so that instead of hardcoding the version number uses a match pattern)? Also, in the CHANGELOG can you please add a reference to this PR?

Sure. no problem.

@Jean85 Jean85 merged commit e9e5ec1 into getsentry:3.5.x Nov 9, 2020
@Jean85
Copy link
Collaborator

Jean85 commented Nov 9, 2020

Thank you for your contribution!

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.

5 participants