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

[ServiceBus] enable beta-trimmed api-extractor dts rollup #21872

Merged
merged 2 commits into from
May 19, 2022

Conversation

jeremymeng
Copy link
Member

in public-trimmed dts rollup, @beta APIs are removed. Because we only ship
public-trimmed version it prevents us from gathering feedback for @beta APIs.
This PR enable generation of beta-trimmed dts rollup.

We still need the piece in the publishing process to use the proper version
depending on whether we are releasing a beta/dev version or not.

in public-trimmed dts rollup, @beta APIs are removed. Because we only ship
public-trimmed version it prevents us from gathering feedback for @beta APIs.
This PR enable generation of beta-trimmed dts rollup.

We still need a piece in the publishing process to use the proper version
depending on whether we are releasing a beta/dev version or not.
@jeremymeng
Copy link
Member Author

@praveenkuttappan I am thinking that we could overwrite service-bus.d.ts with the beta version when publishing beta/dev versions. Something like below before pack

if (!GA) {
  read the two paths from api-extractor.json
  if ($betaTrimmedFilePath) {
    copy $betaTrimmedFilePath $publicTrimmedFilePath
  }
}

Is this possible? Or anyone has other suggestions?

@xirzec
Copy link
Member

xirzec commented May 13, 2022

@praveenkuttappan I am thinking that we could overwrite service-bus.d.ts with the beta version when publishing beta/dev versions. Something like below before pack

if (!GA) {
  read the two paths from api-extractor.json
  if ($betaTrimmedFilePath) {
    copy $betaTrimmedFilePath $publicTrimmedFilePath
  }
}

Is this possible? Or anyone has other suggestions?

This is interesting. I know Office (and rush itself) publishes special "plusbeta" versions of their packages that contain the beta surface area. Maybe after you collect feedback from EngSys and folks on the team we can schedule an arch board? I'd be curious for feedback from the architects on this one.

also /cc @bterlson

@azure-sdk
Copy link
Collaborator

API change check for @azure/service-bus

API changes are not detected in this pull request for @azure/service-bus

@jeremymeng
Copy link
Member Author

I know Office (and rush itself) publishes special "plusbeta" versions of their packages that contain the beta surface area.

Yeah, I forgot to add the link to the doc https://api-extractor.com/pages/setup/configure_rollup/. It's recommended either updating typing field (causing package.json code churn), or overwriting

@jeremymeng jeremymeng requested a review from mpodwysocki May 13, 2022 22:32
@joheredi
Copy link
Member

This is interesting and I like it overall! There are other teams like Cosmos which need Beta api management and I think we can come up with general guidance using this approach.

Is our current structure alpha, beta and stable sufficient or do we need another class like Jeff mentioned?

I think there are some customers that are interested in trying out some Beta features but the rest of the library needs to be stable, here's where a new class of relases will be useful plusBeta (mostly new apis the rest mirros GA). While beta may contain changes that could make the library less stable.

@jeremymeng
Copy link
Member Author

Is our current structure alpha, beta and stable sufficient or do we need another class like Jeff mentioned?

Current we use alpha package versions for daily builds. Nothing is related to API status. We use beta version numbers for preview. We are not really using @beta doc tag currently. The GA versions are stable releases.

If we are adding another class of preview version:

  • -beta for previewing features/APIs that are closed to GA (we could still make API changes)
  • -plusbeta for previewing a handful of APIs that are subject to change?

I am not sure whether we need the distinctions right now.

Another question is that the @beta APIs can still be used in JavaScript, even it's not exposed in the typing. Do we consider them as public API surface or not?

@jeremymeng
Copy link
Member Author

I am thinking that we could overwrite service-bus.d.ts with the beta version when publishing beta/dev versions.

Thinking a bit more, now I feel that we should have the source code to be consistent (as much as we can) with the published package so updating package.json sounds better to me.

@jeremymeng
Copy link
Member Author

  • -plusbeta for previewing a handful of APIs that are subject to change?

-plusbeta is more for previewing service features that may be in Beta for a very long time. Most api surface is stable, but a few APIs will be in beta status.

@jeremymeng
Copy link
Member Author

So for beta releases we want to use the beta-trimmed version. For GA releases, if we want to surface @beta APIs, we would also need to use the beta-trimmed version.

@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@deyaaeldeen deyaaeldeen left a comment

Choose a reason for hiding this comment

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

It makes to me to do this for a beta release.

For the stable release, I would vote to not use the beta trimmed typing.

@xirzec
Copy link
Member

xirzec commented May 17, 2022

I was talking to @KarishmaGhiya about this - I think one potential solution is to augment our release pipeline to do something like:

  1. Create a GA package as normal
  2. Update package version to include -beta (e.g. 2.3.1 becomes 2.3.1-beta), update typings to point to the beta trimmed version, then pack this as a second artifact
  3. Publish both artifacts, making sure to publish the non-beta version first (so semver won't resolve to the beta version)

Does this sound workable?

/cc @mpodwysocki @bterlson

@jeremymeng
Copy link
Member Author

3. Publish both artifacts, making sure to publish the non-beta version first (so semver won't resolve to the beta version)

Will this address Cosmos team's desire of using beta api in GA packages? I thought they wanted to ship the whole library as GA with some Beta features included.

@xirzec
Copy link
Member

xirzec commented May 17, 2022

Will this address Cosmos team's desire of using beta api in GA packages? I thought they wanted to ship the whole library as GA with some Beta features included.

Cosmos specifically might decide to not exclude beta from their GA packages rather than publishing a separate version, but I think that will be an exception for them rather than what we'd recommend as best practice.

@jeremymeng
Copy link
Member Author

I am merging this PR so we can release a package. We should continue the discussion either here or in a new issue.

@jeremymeng jeremymeng merged commit 81a37f1 into Azure:main May 19, 2022
@jeremymeng jeremymeng deleted the sb/enable-beta-trimmed-dts-rollup branch May 19, 2022 18:07
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.

6 participants