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

Update CA2255 description per documentation team review #5733

Merged

Conversation

jeffhandley
Copy link
Member

Fixes #5732, applying wording feedback received while publishing the documentation for this analyzer. dotnet/docs#27096 (review)

@jeffhandley jeffhandley requested a review from a team as a code owner November 18, 2021 22:39
@codecov
Copy link

codecov bot commented Nov 18, 2021

Codecov Report

Merging #5733 (83cfeeb) into main (64a8218) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5733   +/-   ##
=======================================
  Coverage   96.03%   96.03%           
=======================================
  Files        1330     1330           
  Lines      306570   306570           
  Branches     9722     9722           
=======================================
+ Hits       294423   294429    +6     
+ Misses       9797     9792    -5     
+ Partials     2350     2349    -1     

@jeffhandley jeffhandley requested a review from buyaa-n November 19, 2021 00:02
Co-authored-by: Buyaa Namnan <buyankhishig.namnan@microsoft.com>
Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM

@Evangelink
Copy link
Member

@jeffhandley You will need to update the xlf files (rebuilding the solution should be enough).

@jeffhandley
Copy link
Member Author

@jeffhandley You will need to update the xlf files (rebuilding the solution should be enough).

Thanks; done!

@@ -2114,7 +2114,7 @@ The logging message template should not vary between calls.

## [CA2255](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2255): The 'ModuleInitializer' attribute should not be used in libraries

Module initializers are intended to be used by application code to ensure an application's components are initialized before the application code begins executing. If library code declares a 'ModuleInitializer' method, it can interfere with application initialization and also lead to limitations in that application's trimming abilities. Library code should therefore not utilize the 'ModuleInitializer' attribute, but instead expose methods that can be used to initialize any components within the library and allow the application to invoke the method during application initialization.
Module initializers are intended to be used by application code to ensure an application's components are initialized before the application code begins executing. If library code declares a method with the 'ModuleInitializerAttribute', it can interfere with application initialization and also lead to limitations in that application's trimming abilities. Library code should therefore not utilize the 'ModuleInitializerAttribute'. Instead of using 'ModuleInitializerAttribute' methods, the library should expose methods that can be used to initialize any components within the library and allow the application to invoke the method during application initialization.
Copy link
Member

Choose a reason for hiding this comment

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

May I suggest to simplify a little the text to something like:

Module initializers are intended to be used by application code to ensure an application's components are initialized before the application code begins executing. If library code declares a method with the 'ModuleInitializerAttribute', it can interfere with application initialization and also lead to limitations in that application's trimming abilities. Instead of using these methods, the library should expose methods that can be used to initialize any components within the library and allow the application to invoke the method during application initialization.

or

Module initializers are intended to be used by application code to ensure an application's components are initialized before the application code begins executing. If library code declares a method with the 'ModuleInitializerAttribute', it can interfere with application initialization and also lead to limitations in that application's trimming abilities. Instead of using methods marked with 'ModuleInitializerAttribute', the library should expose methods that can be used to initialize any components within the library and allow the application to invoke the method during application initialization.

@jeffhandley
Copy link
Member Author

Gosh; this PR completely fell off my radar to finish up. I applied the 2nd suggestion from @Evangelink and resolved the merge conflicts. Once merged, I'll circle back to the docs repo to apply that wording suggestion there too.

@jeffhandley jeffhandley requested a review from Evangelink May 2, 2022 19:20
@jeffhandley jeffhandley enabled auto-merge (squash) May 2, 2022 19:21
@Youssef1313
Copy link
Member

@jeffhandley Build is failing due to incorrect RulesMissingDocumentation.md. The suggestion should fix it.

Co-authored-by: Youssef Victor <youssefvictor00@gmail.com>
@jeffhandley
Copy link
Member Author

@jeffhandley Build is failing due to incorrect RulesMissingDocumentation.md. The suggestion should fix it.

Thanks! Looks like my merge was done incorrectly.

@jeffhandley jeffhandley merged commit 491f1ed into dotnet:main May 3, 2022
@github-actions github-actions bot added this to the vNext milestone May 3, 2022
@jeffhandley jeffhandley deleted the jeffhandley/ca2255-text-changes branch May 3, 2022 05:11
@jmarolf jmarolf modified the milestones: vNext, .NET 7.x Nov 15, 2022
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.

Update wording of CA2255 description
6 participants