Skip to content

Conversation

@alundiak
Copy link
Collaborator

@alundiak alundiak commented Feb 18, 2017

  1. Even, require-css doesn't have devDependencies (so far, maybe in future something changed), we better to ignore node_modules and bower_components. This is valuable at least for use case, mentioned in README.md file in regards to installing csso locally and running npm test which requires local requirejs. Also ignored DS_Strore and .log

  2. During command r.js -o example/build.js it's also checking node_modules and bower_components folders, if such exists. So I added to exclude pattern in example/build.js.

  3. npm has dedicated script test, which can be valuable during CI, and just for simplification and usability as developer not to type lot of text. Added alias in package.json.
    But FYI: for running node test/test.js u need to install requirejs locally, because global is not accessible. In regards to this small note, I may create separate PR later.

  4. bower.json has not valid structure, which cause warning during any bower activity:

bower invalid-meta for:/Users/alund/prj/require-css/bower.json
bower invalid-meta The "main" field has to contain only 1 file per filetype; found multiple .js files: ["css.js","css-builder.js","normalize.js"]

So I changed to be only css.js

  1. I personally like to use tab spacing, but I see, that majority of code is indented by using 2 spaces. So I added .editorconfig and .jsbeautifyrc which will prevent from wrong indentation. Also in future, I will create grunt task for auto-jsbeautify before commit.

@alundiak alundiak changed the title Small technical but useful changes [.gitignore, package.json, test folder] Small dev. changes [.gitignore, package.json, .editorconfig, .jsbeautifyrc] Feb 18, 2017
@alundiak alundiak merged commit 1778912 into guybedford:master Feb 18, 2017
[*]
indent_style = space
indent_size = 2
trim_trailing_whitespace = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

trim_trailing_whitespace = true will be removed. It's not very useful.

indent_style = space
indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In many cases, this insert_final_newline = true expected by github, so better to have it automated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant