witoldsz / angular-http-auth

MIT License
2.38k stars 418 forks source link

Features/grunt task #123

Closed gitarno closed 7 years ago

gitarno commented 8 years ago

112

witoldsz commented 7 years ago

Is it really necessary to create like 3 files with more text than the actual source code, 4 dependencies, including excessive build tool, forcing me to install it and all this in 5 commits, just to minify one file?

Are you sure it cannot be done with just one extra line in "scripts" section and one in "devDependencies" in package.json? No grunt, no extra CLI, no extra wrappers. Come on…

BTW: I am not familiar with NPM package manners: when are contributors supposed to create new minified version? On each commit, each pull request or just once in a while, just before release?

simison commented 7 years ago

This should do if you're interested —

npm i uglify-js --save-dev

At scripts:

scripts: {
  'minify': 'uglifyjs src/http-auth-interceptor.js -o dist/http-auth-interceptor.min.js -c'
}
spengilley commented 7 years ago

I appreciate the feedback. NPM does way more than I knew :)

I have made some changes based on @simison suggestions. This works lovely except that if the list folder doesn't exist it errors. I can't commit an empty list folder. Are you happy for it to out the .min.js file into src?

I haven't committed these changes yet though. Will await a response here

simison commented 7 years ago

mkdirp it ;-)

edit; or actually creating a minified dist file needs to work on unix systems only, right?

mkdir dist && uglifyjs...
gitarno commented 7 years ago

On the issue of testing, I use it on personal projects, but it is optional.

The proposal to put the 4 modules of NPM is exclusively to have a future management about the tasks that can be put, such as angular-injector ([gulp | grunt] -ng-annotate), documentation, etc ...

The modules: Grunt Grunt-contrib-uglify: as everyone will comment above, for JS minification

Load-grunt-config: separates the project-based grunt / task modules like twitter-bootstrap and another that end up with this approach to modular gruntfile development

Time-grunt: only to manage and return a report of how long it took to execute each task (optional)

I had proposed to make available a minified files by thinking that: The particularity of building some development may hinder the life of the developer on the other side who will have to manage the packages that it has as dependence ends up causing a series of complexity due to "N" dependencies, sometimes.

spengilley commented 7 years ago

I have posted an update to #139. The process is now more streamlined as suggested

simison commented 7 years ago

BTW: I am not familiar with NPM package manners: when are contributors supposed to create new minified version? On each commit, each pull request or just once in a while, just before release?

You'd create it just prior to pushing a new version, just like you'd bump version at package.json, too only by yourself. PRs shouldn't include minified versions nor version bumps.

Something like this could be useful?

scripts: {
  'build': 'mkdir dist && cp src/http-auth-interceptor.js dist && uglifyjs dist/http-auth-interceptor.js -o dist/http-auth-interceptor.min.js -c',
  'version': 'npm run build && git add -A dist',
  'postversion': 'git push && git push --tags'
}

🚨 Didn't test run this and frankly it might be a bit too much.

And if we would have tests, you could add: 'preversion': 'npm test'

Read more: https://docs.npmjs.com/cli/version

:skull: ...now all this said, I'm a minified file rebel: https://github.com/witoldsz/angular-http-auth/pull/139#issuecomment-259445384

simison commented 7 years ago

The proposal to put the 4 modules of NPM is exclusively to have a future management about the tasks that can be put

@gitarno as this package is just 135 lines of code, frankly all that sounds a bit overkill.

witoldsz commented 7 years ago

Merged grunt/gulp-free version. Thanks for bringing the subject to the table (again) :-)