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

Remove deprecated nimble service #2822

Closed
wants to merge 2 commits into from
Closed

Remove deprecated nimble service #2822

wants to merge 2 commits into from

Conversation

wty-Bryant
Copy link
Contributor

No description provided.

@wty-Bryant wty-Bryant requested a review from a team as a code owner October 7, 2024 04:24
Copy link
Contributor

@Madrigal Madrigal left a comment

Choose a reason for hiding this comment

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

If available, we should link any public announcement on these service deprecations

"type": "feature",
"description": "Remove deprecated `nimble` service.",
"modules": [
"."
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: shouldn't this be scoped to nimble instead of .?

Copy link
Contributor

@lucix-aws lucix-aws Oct 7, 2024

Choose a reason for hiding this comment

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

So technically the nimble module is being deleted -- I'm not sure if our tooling is written to handle that as an edge case. I suspect it will try to go update service/nimble/CHANGELOG.md to include this new entry, but that file wouldn't exist anymore since we deleted it, so the release would probably explode.

The other problem we have (recently noted in #2819, not directly although I think this is another case of that) is that a changelog like this bumps the root SDK module for basically no reason (no actual change) which by proxy bumps every service client module for equally no reason.

TL;DR not sure what to do here. Let's discuss internally. We should honestly probably just not have a changelog for these types of deprecations for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

involved modules are generated by changelog cmd according to which commit we use, here I passed the commit including both model and service api removal

Copy link
Contributor

@lucix-aws lucix-aws left a comment

Choose a reason for hiding this comment

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

Let's not merge this until we make a decision on ☝️

@wty-Bryant wty-Bryant closed this Oct 7, 2024
@wty-Bryant wty-Bryant deleted the feat-remove-nimble branch October 7, 2024 18:37
@wty-Bryant wty-Bryant restored the feat-remove-nimble branch October 7, 2024 18:38
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.

3 participants