-
Notifications
You must be signed in to change notification settings - Fork 85
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
Deprecate diff chain features #759
Conversation
#[deprecated(since = "0.5.0", note = "diff chain features are slated for removal")] | ||
#[must_use] | ||
pub fn force_integration_update(mut self, force: bool) -> Self { | ||
self.force_integration_update = force; |
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.
Shouldn't this just be removed? If the account always publishes integration updates, then this flag becomes useless. Compared to 0.4
this wouldn't even be breaking, because we added PublishOptions
after that release.
Raises the question whether our Breaking Change
label is relative to the previous release or relative to the last "proper" release (0.4
in this case). Would be weird actually, to add a method that is deprecated from the start.
As far as I can see only the test_account_publish_options_force_integration
would have to be removed, so it shouldn't have to many compounding effects.
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.
The force_integration_update
still has the effect of force-publishing even if there are no changes, so I'm unsure if we should remove it entirely or replace it with something like force_publish
. The point is that because it was decided not to remove the diff chain (yet), there is the chance that all these features will be un-deprecated, which would make force_integration_update
immediately useful again as it currently stands.
Breaking change
is relative to the last non-dev release.
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.
Since we might re-enable diff chain features, which might make force_integration_update
useful again, I'll approve 👍.
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! The JSDoc comment is the way to go.
Description of change
Deprecates all diff chain features, structs, and functions. This is mostly achieved by deprecating the
identity-diff
crate and entirediff
modules where possible, which marks all imports from those paths as deprecated.Note that, as discussed, this also forces all
Account
updates to publish to the integration chain to prevent a migration step for the majority of users.The
diff_chain
example has been removed along with the diff updates inresolve_history
to discourage their use but the documentation/wiki retains references to the diff chain, along with unit tests, in case we choose un-deprecate the feature in the future.Rust
In Rust the deprecation annotations cause compilation warnings (which we ignore internally with
#![allow(deprecated)]
).and deprecated functions are underlined:
Javascript
In Javascript the
@deprecated
doc-comment causes the IDE to strikethrough the function. I'm not aware of any compilation warning for Typescript though, unless it can be enabled with a lint? Is there anything more we should do here?Links to any relevant issues
Fixes #703.
Type of change
How the change has been tested
Tests and examples pass locally.
Change checklist