Skip to content

Conversation

@christothes
Copy link
Member

This clarification came out of some on-boarding feedback. Please validate that the statements are correct.

@christothes christothes requested a review from pakrym April 28, 2020 23:21
CONTRIBUTING.md Outdated
## Public API additions

If you make a public API addition, the `eng\scripts\Export-API.ps1` script has to be run to update public API listings.
If you make a public API addition, the `eng\scripts\Export-API.ps1` script has to be run to update public API listings. This generates a file in the library's directory similar to the example found in `/sdk/template/Azure.Template/api/Azure.Template.netstandard2.0.cs`. Note that this file is not required if each csproj in the library has the `EnableApiCompat` parameter set to false.
Copy link
Contributor

Choose a reason for hiding this comment

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

The API listing file is useful even when there was no release yet/library is in preview. It allows reviewers to easily see the public API changes made.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps I should reword it to say ‘prior to preview’?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for doing this. I agree with @pakrym and see the value in this for the reviewers too, I think the part that we can make more clear is that prior to having public packages, EnableApiCompat should be set to false, but Export-API still has to be called to generate the apis. Once there are public packages, EnableApiCompat should be flipped to true and the reference to that package should be added to ApiCompat.csproj.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've made some tweaks to the wording. Does that sound better?

@weshaggard weshaggard merged commit 8b31ec0 into Azure:master Apr 29, 2020
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