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

Changed the access modifier of the AutowireViewModel method to public #2557

Merged
merged 3 commits into from
Sep 10, 2021
Merged

Conversation

tstephansen
Copy link
Contributor

Description of Change

Changed the access modifier of the AutowireViewModel method from internal to public in the MvvmHelpers class and added documentation.

Bugs Fixed

API Changes

Changed:

  • internal static void AutowireViewModel => public static void AutowireViewModel

Behavioral Changes

Developers can now access this method from an external assembly.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard

@dnfadmin
Copy link

dnfadmin commented Sep 10, 2021

CLA assistant check
All CLA requirements met.

@tstephansen
Copy link
Contributor Author

Should the access modifier for the UWP/WinUI method be changed to public as well?

@brianlagunas
Copy link
Member

Might as well, so the API is consistent across platforms.

Copy link
Member

@dansiegel dansiegel left a comment

Choose a reason for hiding this comment

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

While I'm fine with the change this should match what we've done for Prism.Forms which is to keep it hidden from Intellisense. It's really an edge case that anyone would need to use it and it isn't something we want people seeing as part of the public API. Be sure to add the following attribute:

[EditorBrowsable(EditorBrowsableState.Never)]

…the AutowireViewModel methods in the MvvmHelpers class.
@brianlagunas brianlagunas merged commit 2b9873d into PrismLibrary:master Sep 10, 2021
@brianlagunas
Copy link
Member

Thanks for the PR

This pull request was closed.
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.

4 participants