Skip to content

Fix different function signature for loadExtension function in docs and code #19669

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

Closed
wants to merge 3 commits into from
Closed

Conversation

m1mir
Copy link

@m1mir m1mir commented Mar 13, 2024

Fixed the argument names for loadExtension usages in the code examples and rewritten the function bodies to use the new names.

Closes #19668

@carsonbot

This comment was marked as outdated.

@carsonbot
Copy link
Collaborator

It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you.

Cheers!

Carsonbot

@m1mir m1mir marked this pull request as draft March 13, 2024 01:44
@m1mir m1mir changed the base branch from 6.1 to 6.4 March 13, 2024 01:54
@m1mir m1mir marked this pull request as ready for review March 13, 2024 02:53
@carsonbot carsonbot added this to the 6.4 milestone Mar 13, 2024
@m1mir m1mir changed the title Fix incorrect argument names for loadExtension examples Fix different function signiature for loadExtension functtion in docs and code Mar 13, 2024
@m1mir m1mir changed the title Fix different function signiature for loadExtension functtion in docs and code Fix different function signiature for loadExtension function in docs and code Mar 13, 2024
@m1mir
Copy link
Author

m1mir commented Mar 13, 2024

I'm aware that CI is failing but, because the intended change and predefined rules are in conflict, I'd like to ask for maintainer input. The reasons for this are the following:

  • The changing or deleting of the rule to accept the changes in this PR would mean that half of the argument_variable_must_match_type rules would be modified in a PR that didn't originally intend to do so.
  • Changing the accepted name would affect the examples of the ExtensionInterface::load function, where the argument name is actually container on the interface.

Removed the rule because it's not consistent with
ConfigurableExtensionInterface::loadExtension
@OskarStark OskarStark changed the title Fix different function signiature for loadExtension function in docs and code Fix different function signature for loadExtension function in docs and code Mar 16, 2024
@OskarStark
Copy link
Contributor

Cc @alamirault

@m1mir
Copy link
Author

m1mir commented May 2, 2024

Fixed in the new code snippet in #19793.

@m1mir m1mir closed this May 2, 2024
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.

3 participants