Remove individual functions deprecated before 0.41 #8992
Conversation
le717
changed the title
|
If you could file an issue with the Extension author, to give them a heads up, then we could merge this cleanup PR. Thanks. |
|
The extension home page goes to a professional website, and I didn't see a contact form (but I'll look again), and the removal may not effect it, as it appeared to be in a direct copy of that Brackets module. I'll also check how the module is referenced. If the extension does not pull the module from core but instead from their copy, then removal should not effect it at all. |
|
@ingorichter I've checked again, the extension is not accessing the If you are ready, I will merge this with master and get it merge-ready again. |
|
@ingorichter I've contacted the extension author about the use of the deprecated function and updated this with master. |
|
|
|
@ingorichter Removed. |
|
Hang on, looks like I removed one too may lines... |
|
@ingorichter There you go, now it's ready to merge. |
| @@ -298,10 +298,6 @@ define(function (require, exports, module) { | |||
|
|
|||
| // document spies | |||
| var deferred = new $.Deferred(); | |||
There was a problem hiding this comment.
I think we don't need the deferred here any longer.
There was a problem hiding this comment.
That's what I thought too, but Travis failed when I removed it because it is referenced in the block at lines 307-320.
There was a problem hiding this comment.
I think we can remove the whole block. I did this locally and all the tests passed.
There was a problem hiding this comment.
If I understand correctly, remove everything from 299-318?
|
310-322 needs to be removed. |
|
Hopefully that is the correct code to be removed. |
|
:-) |
|
Tests pass on my machine. Thanks. |
Remove individual functions deprecated before 0.41
|
@le717 Fwiw, I looked at that "relution.brackets" extension and they are actually using the Brackets core API that was removed here -- not their own copy. See line 113 where the value of Though fwiw this is another one where we gave extension authors zero warning before deleting the API... :-( We need to be more careful about late-game API removals in the future. |
|
I sent the author an email via their contact form warning them about this and even linked back to this issue. Go back in hole because of all the the breakage I've created |
|
@le717 No worries, it's our job to review too! |
|
@stefanbuck & @stefanjauker -- I notice you're associated with 'mwaylabs' on GitHub. Are you part of the team that maintains the "Relution.io Brackets plugin"? Please be aware that the "preview" functionality in this extension will be broken in the next release of Brackets (1.0), due to the API that was removed in this pull request. (@le717 left a note via your online contact form, but I figured I'd ping you here in case that message didn't reach you). Note that Brackets 1.0 will be released in 3-4 days. |
|
What a mess... I didn't expect that many issues after the initial assessment. :-( |
|
@peterflynn Thanks for the hint. We will refactor the plugin on monday. |
|
@stefanjauker That's great, thanks! Sorry for the trouble. |
More towards #8751.
The following APIs are removed with this PR, listed along side the commit they were deprecated with and the release that deprecation went into effect.
FileUtils.canonicalizeFolderPath()(a9ab07c, 0.33)LiveDevelopment/Agents/CSSAgents.getStylesheetURLs()(04b015e, 0.37)LiveDevelopment/Documents/CSSDocument.getStyleSheetFromBrowser()(d2a8a82, 0.37)ProjectManager.isBinaryFile()(0c02184, 0.39)There is only instance any of these APIs are used, and that is
getStylesheetURLs()in therelution.bracketsextension by http://www.relution.io/en/, and that is because they have duplicated that module in their extension (possibly with changes just for them).