-
Notifications
You must be signed in to change notification settings - Fork 468
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
Fix #7008 Make AnalyzerOptionsExtensions to have public modifier instead of internal #7085
Fix #7008 Make AnalyzerOptionsExtensions to have public modifier instead of internal #7085
Conversation
After I compiled on my local, I got some errors that says some objects need to be modified to be public:
Should I make |
That sounds reasonable to me. |
Thanks! I will submit next commit with those types marked as |
Done setting classes to public. @mavasani do I have to update the |
b134b13
to
65d287d
Compare
Yes, please use the code fix to add the entries to |
Done committing updated PublicAPI.Unshipped.txt file. |
81126e6
to
1df41d4
Compare
Done resolving merge conflict. Now it's ready to review, @mavasani |
@mavasani For example: (as shown in the Azure Pipelines https://dev.azure.com/dnceng-public/public/_build/results?buildId=513774&view=logs&j=f0f0520d-467b-5d1d-a95d-4e3a2581fe8a&t=c140c00f-6c0a-5356-d099-4e047a5525ed&l=163)
Ok, I'm going to try to resolve this error, I'll update you again when it's done. |
I need some help on how to resolve the compile error on this project of Analyzer.Utilities.Unittests project, because the class name and the namespace on both Microsoft.CodeAnalysis.NetAnalyzers and Test.Utilities references are exactly the same, as they are actually pointing to the same source file. Looking at the references of Analyzer.Utilities.Unittests project, it has references to other projects that has the same shared project references: IMHO, this is one of the possible reason that cause the build error of having "error CS0433" since both projects has the same ref to the same shared projects, the "Analyzer.Utilities". Feel free to correct me, though. Is there any guidance I should do to fix this errors? |
Pardon, please review and let me know how to proceed further, as I'm currently having the problem of errors in #7085 (comment) Thanks in advance! |
Just to remind, could you help me to solve the compile problem I've described in #7085 (comment) ? Or should I update the Test.Utilities.csproj to remove the line of Import Project Analyzer.Utilities.projitems and add project reference to Microsoft.CodeAnalysis.NetAnalyzers instead? Please confirm. |
@eriawan We need these types to be public only when included in the #if ANALYZER_UTILITIES
public class Unit
#else
internal class Unit
#endif
{
... class definition
} And define the preprocessor directive |
@mavasani I'm going to do this today 🙂 |
5366322
to
4487c21
Compare
…csproj and update AnalyzerOptionsExtensions, SymbolNamesWithValueOption, Unit with #if for !TEST_UTILITIES and TEST_UTILITIES as #else
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7085 +/- ##
==========================================
- Coverage 96.44% 96.43% -0.01%
==========================================
Files 1413 1413
Lines 337719 337719
Branches 11177 11177
==========================================
- Hits 325700 325694 -6
- Misses 9200 9206 +6
Partials 2819 2819 |
Done updating TestUtilities.csproj to add constant/symbol and also update some classes to have Please review, thanks in advance! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Fixes #7008
Make AnalyzerOptionsExtensions to have public modifier instead of internal