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

Public api analyzer [API-1573] #730

Merged
merged 9 commits into from
Dec 9, 2022

Conversation

zpqrtbnk
Copy link
Contributor

@zpqrtbnk zpqrtbnk commented Sep 28, 2022

This PR adds the Microsoft.CodeAnalysis.PublicApiAnalyzers analyzer to the Hazelcast.Net project. These analyzers maintain a signature of the complete public API exposed by the project, in a set of text files, and issue warnings whenever the current signature does not match the files.

This means that... if a public method or property or class or anything is changed, added, removed, anything - a warning will be logged in the build. This is used by the Roslyn team to monitor their API changes, and it willbe greatly beneficial to us to ensure compatibility between versions and track changes.

Note that the PR relies on a beta version of the analyzers that support merging multiple definition files (see PR#5422) and this is considered acceptable since it is a development-time dependency, and does not impact the production code in any ways.

How it works

Development

Covered projects (Hazelcast.Net at the moment) have an extra src/Hazelcast.Net/PublicAPI directory that contains a set of files. The PublicAPI.Shipped.txt files define the signature of the public API. Whenever that public API is modified, i.e. whenever the code does not match that signature anymore, RS0016 and other warnings appear during the build.

Whenever a change is made that is not a mistake, it should be added to PublicAPI.Unshipped.txt files. This will get rid of the warnings, but the change is still considered "proposed" and not "shipped" (see Release section below). Beware of conditional code (code within #if blocks) and their corresponding PublicAPI.Unshipped.txt files.

At one point of time, and before entering the release phase, the PublicAPI.Unshipped.txt should be carefully reviewed and merged into the PublicAPI.Shipped.txt files. Ideally these reviews should be independent PRs.

Release

When building a release/* branch, RS0016 and other warnings are escalated to errors (rule in AnalysisRules.prop) and therefore fail the build. In addition the hz.ps1 script looks for PublicAPI.Unshipped.txt files and fails the build in case some are detected.

A release build can therefore succeed only if changes to the public API have been reviewed, merged, etc.

Review

Open any public options file such as MetricsOptions and add a public property such as public int Whatever { get; set; }. Rebuild, ensure you see a new warning. Name the current branch release/foo and rebuild with hz.ps1 build, ensure the build fails.

Modify the PublicAPI.Unshipped.txt file to declare the new option. Rebuild, ensure the warning is one. Switch to release/foo and rebuild with hz.ps1, ensure the buid fails.

Merge the PublicAPI.Unshipped.txt changes into PublicAPI.Shipped.txt. Rebuild, ensure you see no warnings. Switch to release/foo and rebuild with hz.ps1, ensure the buid succeeds.

References

@zpqrtbnk zpqrtbnk changed the title Public api analyzer Public api analyzer [API-1573] Sep 28, 2022
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch from 7e7f56f to cb53a7a Compare October 5, 2022 11:04
@codecov-commenter
Copy link

codecov-commenter commented Oct 5, 2022

Codecov Report

Merging #730 (c5fd6f3) into master (17f138a) will increase coverage by 0.18%.
The diff coverage is 98.08%.

@@            Coverage Diff             @@
##           master     #730      +/-   ##
==========================================
+ Coverage   83.41%   83.60%   +0.18%     
==========================================
  Files         862      872      +10     
  Lines       20113    20418     +305     
==========================================
+ Hits        16777    17070     +293     
- Misses       3336     3348      +12     
Impacted Files Coverage Δ
...t.Net/Serialization/Compact/SchemaBuilderWriter.cs 100.00% <ø> (ø)
...et/Serialization/SerializationService.ReadWrite.cs 78.00% <45.45%> (-3.06%) ⬇️
src/Hazelcast.Net/Core/ConvertEx.cs 83.33% <83.33%> (ø)
...lization/Compact/CompactDictionaryGenericRecord.cs 96.66% <96.66%> (ø)
...ct/CompactDictionaryGenericRecord.GeneratedCode.cs 100.00% <100.00%> (ø)
.../Serialization/Compact/CompactGenericRecordBase.cs 100.00% <100.00%> (ø)
...mpact/CompactGenericRecordBuilder.GeneratedCode.cs 100.00% <100.00%> (ø)
...rialization/Compact/CompactGenericRecordBuilder.cs 100.00% <100.00%> (ø)
...lization/Compact/CompactGenericRecordSerializer.cs 100.00% <100.00%> (ø)
...n/Compact/CompactReaderExtensions.GeneratedCode.cs 100.00% <100.00%> (ø)
... and 13 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ihsandemir ihsandemir added this to the 5.2.0 milestone Oct 10, 2022
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch 9 times, most recently from e800298 to c0f4ded Compare November 4, 2022 09:14
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch 4 times, most recently from 1bcecda to 2e711eb Compare November 17, 2022 16:43
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch 3 times, most recently from 18f59c1 to 92864d7 Compare November 22, 2022 18:25
@zpqrtbnk zpqrtbnk requested review from srknzl and emreyigit November 23, 2022 08:43
@zpqrtbnk zpqrtbnk marked this pull request as ready for review November 23, 2022 08:44
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch from 92864d7 to fc5eca9 Compare November 23, 2022 14:08
@srknzl srknzl removed their request for review November 24, 2022 10:07
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch 2 times, most recently from 05c93b6 to f05d290 Compare November 29, 2022 11:17
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch from f05d290 to 362f7d1 Compare December 2, 2022 15:36
@zpqrtbnk zpqrtbnk force-pushed the public-api-analyzer branch from bc533bd to ade657c Compare December 7, 2022 18:47
Copy link
Collaborator

@emreyigit emreyigit left a comment

Choose a reason for hiding this comment

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

Looks nice.

@zpqrtbnk zpqrtbnk merged commit c89b29c into hazelcast:master Dec 9, 2022
@zpqrtbnk zpqrtbnk deleted the public-api-analyzer branch December 9, 2022 16:19
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