ENH Remove code that had been retained for backwards compatibility #1883
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
GuySartorelli
commented
Jan 9, 2025
Comment on lines
-105
to
-111
| // pass in a single icon (retain backwards compatibility) | ||
| icon: PropTypes.shape({ | ||
| nodeName: PropTypes.string, | ||
| className: PropTypes.string, | ||
| onClick: PropTypes.func, | ||
| action: (props) => { if (props.action) { throw new Error('action: no longer used'); } }, | ||
| }), |
There was a problem hiding this comment.
This isn't used anywhere that I can find - and if anyone's using it for their own uses they can just pass it as an array to the icons prop instead.
Comment on lines
-610
to
-618
| // these two functions retained for backwards compatibility | ||
| showList: function(instant) { | ||
| this.showGroupsList(instant); | ||
| this.showMembersList(instant); | ||
| }, | ||
| hideList: function(instant) { | ||
| this.hideGroupsList(instant); | ||
| this.hideMembersList(instant); | ||
| }, |
There was a problem hiding this comment.
These were only kept in case someone was calling them directly in their own code. See #1531 (review)
Now that we're doing a major release, we can remove these unnecessary functions.
| // | ||
| // Example: protected function updateUsage(ArrayList &$usage, DataObject &$record) | ||
| // $dataObjects = $usage->exclude('ClassName', MyDataObject::class); | ||
| $this->extend('updateUsage', $usage, $record); |
There was a problem hiding this comment.
Not used in any core or supported modules
Comment on lines
-2300
to
-2322
| /* src/jquery.entwine.legacy.js */ | ||
|
|
||
| (function($) { | ||
|
|
||
| // Adds back concrete methods for backwards compatibility | ||
| $.concrete = $.entwine; | ||
| $.fn.concrete = $.fn.entwine; | ||
| $.fn.concreteData = $.fn.entwineData; | ||
|
|
||
| // Use addHandler to hack in the namespace.$.concrete equivilent to the namespace.$.entwine namespace-injection | ||
| $.entwine.Namespace.addHandler({ | ||
| order: 100, | ||
| on: function(selector, k, v) { return false; }, | ||
|
|
||
| namespaceMethodOverrides: function(namespace){ | ||
| namespace.$.concrete = namespace.$.entwine; | ||
| namespace.injectee.concrete = namespace.injectee.entwine; | ||
| namespace.injectee.concreteData = namespace.injectee.entwineData; | ||
| return {}; | ||
| } | ||
| }); | ||
|
|
||
| })(jQuery); |
There was a problem hiding this comment.
concrete was a very very very very very old way of invoking entwine. There's virtually 0 chance anyone is using it.
|
I don't think anything here needs to be mentioned in the changelog but happy to add if someone disagrees. |
emteknetnz
approved these changes
Jan 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We don't need to retain this compatibility anymore. Some of this is dead code, and some is just tech debt that should have been properly deprecated.
Issue