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

Require PathUtils instead of using the global one #12203

Merged
merged 1 commit into from
May 2, 2016
Merged

Require PathUtils instead of using the global one #12203

merged 1 commit into from
May 2, 2016

Conversation

ficristo
Copy link
Collaborator

@ficristo ficristo commented Feb 6, 2016

Actually I decided to try again: similarly to #11734 but this time using the submodule.

MiguelCastillo
MiguelCastillo reviewed Feb 13, 2016
View reviewed changes
src/brackets.js
// interim.
var PathUtils = require("thirdparty/path-utils/path-utils");

Object.defineProperty(window, "PathUtils", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. Thank you!

@MiguelCastillo
Copy link
Contributor

These changes look good. When we add breaking changes, we tend to notify extension authors. @marcelgerber, do you think you can help here please?

@petetnt petetnt mentioned this pull request Feb 21, 2016
Open
@marcelgerber
Copy link
Contributor

There are quite a few extensions that still use the global object:

  • aem-sightly-brackets-extension
  • brackets-latex
  • cf10.cfbrackets.org
  • cf11.cfbrackets.org
  • cf7.cfbrackets.org
  • cf8.cfbrackets.org
  • cf9.cfbrackets.org
  • luceebrackets.org
  • nicolo-ribaudo.brackets-zip
  • obd3.cfbrackets.org
  • railo2.cfbrackets.org
  • railo3.cfbrackets.org
  • railo4.cfbrackets.org

But, as I see this, this is not a breaking change, as the global object is still accessible, it just outputs an deprecation warning when used. (Am I right?)

@ficristo
Copy link
Collaborator Author

ficristo commented May 1, 2016

Yes, this PR come with a deprecation warning, it shouldn't break extensions.

@marcelgerber
Copy link
Contributor

Same as the Mustache one, this one looks fine. Just that one thing that you could remove the Gruntfile line goes for this, too.

@ficristo
Copy link
Collaborator Author

ficristo commented May 1, 2016

@marcelgerber done.

@marcelgerber
Copy link
Contributor

Ah, your Mustache changes don't go well together with this one.

@ficristo
Copy link
Collaborator Author

ficristo commented May 1, 2016

Fixed (I hope).

@marcelgerber
Copy link
Contributor

Looks good now. Thank you!

@marcelgerber marcelgerber merged commit 81375bb into adobe:master May 2, 2016
@ficristo ficristo deleted the require-path-utils branch May 2, 2016 12:40
marcelgerber pushed a commit that referenced this pull request May 19, 2016
Fix Extension Manager test failures
ce12a86 
Caused by interference between #12389 and #12203.
The former utilizes PathUtils, while the latter adapted PathUtils
to our normal require() workflow.
@core-ai-bot core-ai-bot mentioned this pull request Aug 30, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ficristo @MiguelCastillo @marcelgerber