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

Requirejs as submodule (text.js & i18n.js as well) #3680

Merged
merged 5 commits into from
May 13, 2013
Merged

Requirejs as submodule (text.js & i18n.js as well) #3680

merged 5 commits into from
May 13, 2013

Conversation

TuckerWhitehouse
Copy link
Contributor

Include RequireJS as a submodule as well as text.js and i18n.js to keep in sync with the repos and make updates easier.

"text" : "../../../thirdparty/text",
"i18n" : "../../../thirdparty/i18n"
"text" : "../../../thirdparty/text/text",
"i18n" : "../../../thirdparty/i18n/i18n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jasonsanjose, should this file not be modified in this situation either?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. It never should be modified. This files is never run. It is just used to test how brackets behave with heavy files from a performance perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, thank you.

@peterflynn
Copy link
Member

@redmunds do you have cycles to review this one? Feel free to reassign if not.

Glenn and I couldn't think of any good reason why we didn't make this a submodule originally, so it seems ok to merge assuming the code changes look good.

@peterflynn
Copy link
Member

@TuckerWhitehouse there are still some whitespace diffs in brackets-concat.js -- can you try to back those out?

Also -- are the SHAs pointing to the same versions of Require, text, and i18n that we had before? Or is this upgrading us to the latest versions?

@ghost ghost assigned redmunds May 10, 2013
@TuckerWhitehouse
Copy link
Contributor Author

@peterflynn I reverted that change to brackets-concat.js (learned a lot about git and rebasing this evening...)
The SHA's are pointing to upgraded versions of each (whatever the latest was when I did the submodule add 9 days ago or so).

  • RequireJS: v2.1.1 >> v2.1.5
  • Text: v1.0.8 >> v2.0.5
  • i18n: v1.0.0 >> v2.0.2

Edit: I added another commit, updating to the latest versions

  • RequireJS: v2.1.5
  • Text: v2.0.6

@jasonsanjose
Copy link
Member

@TuckerWhitehouse can you update your pull request? There's a merge conflict.

@TuckerWhitehouse
Copy link
Contributor Author

@jasonsanjose I think I fixed it (Conflict in NOTICE for some odd reason?)

@jasonsanjose
Copy link
Member

Looks like the latest errors from Travis are due to connectivity to GitHub and not any errors from running grunt.

@redmunds
Copy link
Contributor

Reviewing...

@redmunds
Copy link
Contributor

This looks good. @jasonsanjose Are you sure Travis is happy? :)

@jasonsanjose
Copy link
Member

Actually, I was able to kick off a new build...I think this is a new Travis feature. Under the settings icon, "Restart Build". Travis is happy now.

@redmunds
Copy link
Contributor

Merging.

redmunds added a commit that referenced this pull request May 13, 2013
Requirejs as submodule (text.js & i18n.js as well)
@redmunds redmunds merged commit 6ea53dd into adobe:master May 13, 2013
@TuckerWhitehouse TuckerWhitehouse deleted the requirejs_submodule branch May 13, 2013 23:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants