-
Notifications
You must be signed in to change notification settings - Fork 142
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
Update remotecfg dep to use alloy-remote-config
over agent-remote-config
#748
Conversation
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally OK with these changes. I'm a little worried that it might be an abrupt change even if we can technically make these kinds of changes for non-stable functionality. Do we have a pulse on how many people this might impact, and if we need to consider a smoother migration?
CHANGELOG.md
Outdated
@@ -10,6 +10,11 @@ internal API changes are not present. | |||
Main (unreleased) | |||
----------------- | |||
|
|||
### Breaking changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clayton-cornell Should we find a different way of classifying these kinds of changes for non-stable functionality now that we're post-1.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe append something like this to the header? Breaking changes (Public preview features)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @richardqlam on this too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I'm not sure further sub-categorizing breaking changes at a top level is all that useful. Feels like a changelog is most consumable when it talks about things on a feature-by-feature level. And then within the context of each feature highlighting what state it is (GA, public preview, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looking at this now. It's still a breaking change, even though it's in a public preview state. The info clearly says it's an update to the public preview.... to my way of thinking, it's clear enough as-is.
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
@rfratto As far as usage we don't have a great pulse other than a couple questions about |
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
PR Description
Closes #783
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist