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

feat(sdk): exported Angular and Beacon client to dedicated package #2183

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

kpanot
Copy link
Contributor

@kpanot kpanot commented Sep 20, 2024

Proposed change

Split of Angular and Beacon API Client to their own dedicated package

Related issues

@kpanot kpanot requested a review from a team as a code owner September 20, 2024 10:56
@kpanot kpanot linked an issue Sep 20, 2024 that may be closed by this pull request
6 tasks
deprecate(sdk): the beacon and angular clients from @ama-sdk/core
Copy link
Member

@vscaiceanu-1a vscaiceanu-1a left a comment

Choose a reason for hiding this comment

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

Could you document this change in the next major version migration guide?

@kpanot
Copy link
Contributor Author

kpanot commented Sep 25, 2024

Could you document this change in the next major version migration guide?

Why?
This PR does not include breaking change and will be integrated in a minor version.
The migration guide should be updated on #2117 and on the v13 (n+2) when removing the deprecated interfaces

@vscaiceanu-1a
Copy link
Member

vscaiceanu-1a commented Sep 25, 2024

Could you document this change in the next major version migration guide?

Why? This PR does not include breaking change and will be integrated in a minor version. The migration guide should be updated on #2117 and on the v13 (n+2) when removing the deprecated interfaces

In the v12 migration guide I would expect some words about this change: the list of deprecations mentioning that they will be removed in v13 and the fact thatng add of new packages will update the imports.

Users don't have to wait v13 until the interfaces will be removed, they can migrate to the new ones in v12. This is why I find this info useful in the v12 migration guide.

@kpanot
Copy link
Contributor Author

kpanot commented Sep 25, 2024

Could you document this change in the next major version migration guide?

Why? This PR does not include breaking change and will be integrated in a minor version. The migration guide should be updated on #2117 and on the v13 (n+2) when removing the deprecated interfaces

In the v12 migration guide I would expect some words about this change: the list of deprecations mentioning that they will be removed in v13 and the fact thatng add of new packages will update the imports.

Users don't have to wait v13 until the interfaces will be removed, they can migrate to the new ones in v12. This is why I find this info useful in the v12 migration guide.

I disagree on proposal to create migration v12 in this PR for the following reasons:

  • Not sure to understand why the deprecation should be listed in migration v12 when it is part of the v11.1
  • I don't think that creating a "migration v12" file in the beginning of v11 is a good idea
  • We discussed previously and already agreed to specify the deprecation in the minor version changelog

If you want to propose this, you can put it in the agenda of the next discussion session.
I don't think it should be a blocker of the merge of this PR, as it is not the first PR with deprecation and, even if the decision is taken to go with this proposal, the migration guide should be created with previous deprecations.

@kpanot kpanot added this pull request to the merge queue Sep 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 25, 2024
@kpanot kpanot added this pull request to the merge queue Sep 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 25, 2024
@kpanot kpanot added this pull request to the merge queue Sep 25, 2024
Merged via the queue into main with commit d4ecdd0 Sep 25, 2024
28 of 29 checks passed
@kpanot kpanot deleted the feature/sdk-client-split branch September 25, 2024 16:38
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.

[Feature]: Split SDK API Client to dedicated packages
4 participants