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

fix: Controller container property assignment #179

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

mkilmanas
Copy link
Contributor

Changes

Please describe both what is changing and why this is important. Include:

  • Removing scope keyword from constructor parameter to avoid property re-declaration
  • In turn that allows compatibility with both symfony version 6.x and 7.x
  • Using given setter to achieve the same desired result

References

[#178]

Testing

Previously Symfony (6.4) app container building (cache:clear) was throwing a fatal error, now it's not.

  • This change adds test coverage

  • This change has been tested on the latest version of Symfony

Checklist

Copy link
Member

@evansims evansims left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @mkilmanas

@evansims evansims merged commit 0f60608 into auth0:main Dec 20, 2023
17 checks passed
@alebedev80
Copy link

@evansims when it will be released?

@evansims
Copy link
Member

Hi, @alebedev80; we're currently in a code freeze until after the holidays.

@alebedev80
Copy link

alebedev80 commented Dec 29, 2023

@evansims last release is broken. why not to release fix asap?

@evansims
Copy link
Member

evansims commented Jan 2, 2024

@alebedev80 As a company policy, we are not permitted to cut releases during a code freeze, as the team is on vacation for the holidays and is unable to provide support. The problematic release was already purged from Packagist to avoid bugged installs. Once our code freeze is lifted, I will cut a release containing the fix as soon as possible.

evansims pushed a commit that referenced this pull request Jan 4, 2024
### Changes

Fix syntax typo in the previous code change in
`AuthenticationController::__construct()`

### References

#178
#179
https://github.com/auth0/symfony/actions/runs/7279226624/job/19835136156
https://github.com/auth0/symfony/actions/runs/7279226624/job/19835137264

### Testing

[ ] This change adds test coverage

[ ] This change has been tested on the latest version of Symfony

### Checklist

[x] I have read the [Auth0 general contribution
guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md)

[x] I have read the [Auth0 Code of
Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)

[ ] All existing and new tests complete without errors
@evansims evansims changed the title Do not redeclare controller container property in order to avoid compatibility issues fix: Controller container property assignment Jan 9, 2024
@evansims evansims mentioned this pull request Jan 9, 2024
evansims added a commit that referenced this pull request Jan 9, 2024
**Fixed**
- Syntax typo in AuthenticationController::__construct()
[\#180](#180)
([mkilmanas](https://github.com/mkilmanas))
- Controller container property assignment
[\#179](#179)
([mkilmanas](https://github.com/mkilmanas))
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.

3 participants