Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Remove CollectionUtils and StringUtils.htmlEscape() #8960

Merged
merged 3 commits into from
Oct 21, 2014
Merged

Remove CollectionUtils and StringUtils.htmlEscape() #8960

merged 3 commits into from
Oct 21, 2014

Conversation

le717
Copy link
Contributor

@le717 le717 commented Sep 4, 2014

Working toward #8751's ultimate goal.

  • The entire utils/CollectionUtils module and StringUtils.htmlEscape() function was deprecated by Add Lo-Dash #5539, which was implemented in Sprint 34. The newest reference to CollectionUtils was in src/brackets.js back in January (471a8c1), which was Sprint 36, but I find nothing on the wiki explaining how long a deprecation cycle is. There are also no tests for nor core usage of CollectionUtils.
  • StringUtils.htmlEscape() has no usage in Brackets core at all.

There is some overlap between this and #8954. It might be best for that to be merged first, unless @marcelgerber wants to make these same changes in that PR.

@marcelgerber
Copy link
Contributor

You're right, it's better if this lands first.
With 196 changed files, I have to merge very often anyway.

@ingorichter
Copy link
Contributor

We need to check if any published extension uses utils/CollectionUtils. Otherwise this extension would be broken.

@le717
Copy link
Contributor Author

le717 commented Sep 4, 2014

@ingorichter I thought about that already. I thought there was an extension by someone on the core team that helped figuring that out, but I can't seem to find it anymore.

EDIT: I think I found it. https://github.com/dangoor/BracketsExtensionGrabber Will comment again with extension report.

EDIT 2: Initial pass of all the uploaded extensions (as gathered by the aforementioned tool) reports no usage of utils/CollectionsUtils, only small usage of StringUtils.htmlEscape() in four extensions:

https://github.com/peterflynn/eval-in-browser
https://github.com/tomsdev/brackets-typescript-code-intel
https://github.com/iwehrman/brackets-simple-js-code-hints
https://github.com/adobe/brackets-edge-web-fonts (PR to fix this at adobe/brackets-edge-web-fonts#174)

Will have results of a more extensive search a little later.

EDIT 3: Through scan complete, no usage of utils/CollectionsUtils found.

@ingorichter
Copy link
Contributor

Sounds good. If none of the Extensions is using utils/CollectionUtils, then we can assume that this is safe to get rid off.
It would be a good idea to file an issue with all 4 extension authors and tell them that StringUtils.htmlEscape has been removed after a long deprecation phase. This will put the ball in their court and we can move on. Sounds good?

@le717
Copy link
Contributor Author

le717 commented Sep 5, 2014

Done. All issues at extension repos have been filed.

@le717
Copy link
Contributor Author

le717 commented Oct 11, 2014

Just updated with master, this is ready to be merged again.

@ingorichter
Copy link
Contributor

Looks good to me. All tests pass on OSX.

ingorichter added a commit that referenced this pull request Oct 21, 2014
Remove CollectionUtils and StringUtils.htmlEscape()
@ingorichter ingorichter merged commit 9400017 into adobe:master Oct 21, 2014
@le717 le717 deleted the remove-collection-utils branch October 21, 2014 18:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants