Conversation
|
@leog please squash your commits into one, and update PR, so that we have clear git history in future. |
leog
force-pushed
the
master
branch
2 times, most recently
from
2d40e91 to
47d09d3
Compare
|
done @alundiak, hope it's what you expected, otherwise let me know |
| { | ||
| "name": "require-css", | ||
| "version": "0.1.4", | ||
| "version": "0.1.8", |
There was a problem hiding this comment.
@leog Why do u update to 0.1.8, when latest master does have this already? Looks like u didn't pull latest master or rebased. Have u? The same with package.json. Ideally, if u pull/rebase this file should NOT be in changed list. Please check.
| global._requirejsCssData.usedBy.css = true; | ||
| } | ||
|
|
||
| if (config.buildCSS != false) { |
There was a problem hiding this comment.
Not sure why this codebase was NOT checked by jshint/eslint/jsbeautifier before, but could u please change to !== usage?
Note:
jshint css-builder.js latest from master:
css-builder.js: line 22, col 16, 'csso' used out of scope.
css-builder.js: line 46, col 11, 'file' is already defined.
css-builder.js: line 144, col 40, Use '===' to compare with '0'.
css-builder.js: line 152, col 4, Missing semicolon.
css-builder.js: line 158, col 4, Missing semicolon.
css-builder.js: line 173, col 8, Missing semicolon.
css-builder.js: line 178, col 25, Use '!==' to compare with 'false'.
css-builder.js: line 195, col 4, Missing semicolon.
css-builder.js: line 220, col 30, Use '!==' to compare with 'false'.
css-builder.js: line 220, col 64, Use '!==' to compare with 'true'.
css-builder.js: line 223, col 23, Use '===' to compare with ''.
css-builder.js: line 227, col 11, Misleading line break before '+'; readers may interpret this as an expression boundary.
css-builder.js: line 234, col 4, Missing semicolon.
| if (config.writeCSSModule && style) { | ||
| if (writeCSSForLayer) { | ||
| writeCSSForLayer = false; | ||
| write(writeCSSDefinition); |
There was a problem hiding this comment.
line 182 looks not very good aligned.
| } | ||
| else { | ||
| cssModule = 'define(function(){})'; | ||
| } |
There was a problem hiding this comment.
line 188 looks not very good aligned.
|
Sorry @alundiak, now it looks better. Please review. |
2 participants
Using same check to see if the current load should be import or link in order to use toUrl API or not respectively. Extracted adding suffix ".css" as common denominator.
Issue #163