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

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

Merged
redmunds merged 5 commits into
adobe:masterfrom
TuckerWhitehouse:requirejs_submodule
May 13, 2013
Merged

redmunds merged 5 commits into
adobe:masterfrom
TuckerWhitehouse:requirejs_submodule

Conversation

@TuckerWhitehouse

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

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

@TuckerWhitehouse

Copy link
Copy Markdown
Contributor Author

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

@jasonsanjose

Copy link
Copy Markdown
Member

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

@redmunds

Copy link
Copy Markdown
Contributor

Reviewing...

@redmunds

Copy link
Copy Markdown
Contributor

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

@jasonsanjose

Copy link
Copy Markdown
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
Copy Markdown
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