Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closure fixes #1551

Merged
merged 1 commit into from
Sep 7, 2015
Merged

Closure fixes #1551

merged 1 commit into from
Sep 7, 2015

Conversation

sgomes
Copy link
Contributor

@sgomes sgomes commented Sep 4, 2015

This PR updates the codebase to play better with the Closure compiler and JSDoc.
It also tweaks the JSCS config, updates the JSCS version and adds a custom linting rule, all in to enforce checks a bit better.

This PR shouldn't break BC.

@addyosmani @samthor PTAL!

@Garbee
Copy link
Collaborator

Garbee commented Sep 4, 2015

LGTM. No BC concerns either, looks like anything that may cause problems isn't supported usage in the first place (since mods are to internal properties.)

@addyosmani
Copy link
Contributor

LGTM. Deferring to @samthor for a final LGTM before we move ahead.

/**
* Function to publish staging or prod version from local tree,
* or to promote staging to prod, per passed arg.
* @param {string} pubScope the scope to publish to.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: single space between @param and {string}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Silly docblockr. Thanks!

@samthor
Copy link
Contributor

samthor commented Sep 4, 2015

LGTM modulo nits. Do you want to get #1547 in too?

@sgomes
Copy link
Contributor Author

sgomes commented Sep 7, 2015

Thanks for the review, @samthor! I'll add those changes in, squash and commit.

I'll review #1547 for you after this, but it's probably best to keep them separate for now. I suspect we'll need yet another PR to fix some things up after they're both merged (we're probably missing some exports/externs).

Also configures JSCS to work with our code a bit better.

Adding license and description to closure-camel-case.js

Some extra fixes

Fixes from @samthor review
sgomes added a commit that referenced this pull request Sep 7, 2015
@sgomes sgomes merged commit 4f52641 into mdl-1.0 Sep 7, 2015
@sgomes sgomes deleted the closure-fixes branch September 7, 2015 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants