Skip to content

Conversation

LeJeanbono
Copy link
Contributor

No description provided.

@LeJeanbono LeJeanbono marked this pull request as ready for review August 20, 2019 10:59
@stof
Copy link
Member

stof commented Aug 20, 2019

The title of the PR does not describe at all what it does...

What it does is adding return types to the generated guard authenticators (for forward-compatibility with Symfony 5)

@LeJeanbono LeJeanbono changed the title Fix deprecation checkCredentials Fix deprecations Aug 20, 2019
@LeJeanbono LeJeanbono changed the title Fix deprecations Fix deprecations "return types" for SF5 Aug 20, 2019
@LeJeanbono LeJeanbono changed the title Fix deprecations "return types" for SF5 WIP : Fix deprecations "return types" for SF5 Aug 20, 2019
@LeJeanbono LeJeanbono changed the title WIP : Fix deprecations "return types" for SF5 Fix deprecations "return types" for SF5 Aug 22, 2019
@romaricdrigon
Copy link
Contributor

Thank you @stof for the explanation.
I have mixed feelings about what to do.

Purely regarding Symfony, return types won't be enforced in 5.0 (see symfony/symfony#33236) and deprecation warnings may start in 5.1, but I'm not totally sure if they will match this PR (for instance, UserInterface does not have any return types at the moment). So there's no rush there.

If we look at this purely from a CS perspective, having return types is nice - as long as Symfony interfaces does not break those later (6.x ?).

What do you think?
Anyhow, @LeJeanbono could you please edit PR title (something like "Added return types"), get rid of the merge commit and squash it?

@romaricdrigon romaricdrigon added Status: Needs Work Additional work is needed Status: Waiting Feedback Needs feedback from the author Related Tests Pass and removed Status: Needs Work Additional work is needed labels Oct 16, 2019
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

The earlier apps move to use return types the better.

@nicolas-grekas nicolas-grekas changed the title Fix deprecations "return types" for SF5 Add return types to generated guard authenticators Oct 20, 2019
@LeJeanbono
Copy link
Contributor Author

Are we ok @romaricdrigon ?

@weaverryan weaverryan changed the base branch from master to main November 16, 2020 20:03
@jrushlow jrushlow added Feature New Feature and removed Status: Waiting Feedback Needs feedback from the author Related Tests Pass labels May 9, 2022
@jrushlow
Copy link
Collaborator

jrushlow commented May 9, 2022

Closing as MakerBundle does not support Guard Authentication anymore. Types have been gradually added in w/ the new security system. Thanks for the work on this @LeJeanbono - I wish we could have gotten to this sooner!

@jrushlow jrushlow closed this May 9, 2022
@LeJeanbono LeJeanbono deleted the fix_deprecations branch May 9, 2022 18:54
@LeJeanbono
Copy link
Contributor Author

You're welcome !!

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

Successfully merging this pull request may close these issues.

5 participants