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

Fixed configuration for authenticator selection criteria #467

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

marcriemer
Copy link
Contributor

Target branch: 4.7.x

  • It is a Bug fix
  • It is a New feature
  • Breaks BC
  • Includes Deprecations

Fixed configuration authenticator selection criteria:

  • Renamed attachment_mode into authenticator_attachment.

@Spomky
Copy link
Contributor

Spomky commented Sep 5, 2023

Hi,

Many thanks for the proposal, but it will break existing applications.
attachment_mode is fine no?

@marcriemer
Copy link
Contributor Author

marcriemer commented Sep 5, 2023

Well, yes that's a problem. But it does not work anyway in 4.7.

The parameters is only used for configuration and does not have any effect, other then throwing warning of a missing array element.

Alternative solution would be to change it everywhere else. Even the unit tests use a different parameter name.

I'll look into this again by tomorrow and make a new proposal with the alternative solutions.

@Spomky
Copy link
Contributor

Spomky commented Sep 6, 2023

OK I found the reason for this issue.
The modification of the option name is acceptable, but this PR needs to be updated to avoid any BC issue.
I will take care of it.

@Spomky Spomky self-assigned this Sep 6, 2023
@Spomky Spomky added the bug Something isn't working label Sep 6, 2023
@Spomky Spomky added this to the 4.7.1 milestone Sep 6, 2023
@nicodemuz
Copy link

@Spomky when are you planning to update the READ-ONLY sub-tree split repos?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants