Skip to content

Update package.json, add package-lock.json, remove unused watch task#62

Open
arzafran wants to merge 1 commit intodevelopfrom
update-dependencies
Open

Update package.json, add package-lock.json, remove unused watch task#62
arzafran wants to merge 1 commit intodevelopfrom
update-dependencies

Conversation

@arzafran
Copy link
Contributor

@arzafran arzafran commented Jan 5, 2018

Background

Dependencies were outdated, and package-lock (introduced in npm 5.0) was missing

Changes done

  • Add package-lock.json
  • Remove watch task from gulpfile (wasn't being used)
  • Update dependencies

@arzafran arzafran self-assigned this Jan 5, 2018
@arzafran arzafran requested review from a user, damianmuti, lcalvino and nazarenooviedo January 5, 2018 13:54
});

// Watch for changes
gulp.task('watch', ['build'], function() {
Copy link

Choose a reason for hiding this comment

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

Although this removal is correct as it was practically duplicated. I'd love to have some 'watch only' feature without the 'serve'. Since there were times I needed to add PHP to the project and work through Apache (opening the project in the browser as http://localhost), the 'serve' in my case was useless, yet the watch is a great thing to have, so you can see the changes by manually refreshing.
Nice change, maybe at some point we can rearrange the tasks having 'run' for build, watch and serve and another task 'watch' as build and watch only (maybe 'run' may call 'watch' and 'serve').

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Nice job!
Future note: I'm not crazy about leaving the package-lock.json file in the project, since I believe this is more useful in a work in progress rather than in a framework for a fresh-start. Maybe we can consider removing it in a future update.

@arzafran
Copy link
Contributor Author

arzafran commented Jan 5, 2018

if we're going to remove the package-lock then we should add it in the .gitignore file, otherwise everytime we develop a new feature for the framework it needs to be manually removed.. but let's debate!

@ghost
Copy link

ghost commented Jan 5, 2018

I'm not sure about that, since once you started with a new project, it's a good thing to share the lock file within the devs, so everybody shares the same npm dependencies. Is just that keeping a lock file that most likely will be outdated when starting a project doesn't make much sense to me. If we add it to the .gitignore, then I'll think it will lose its purpose. But that's my humble opinion.

@arzafran
Copy link
Contributor Author

arzafran commented Jan 5, 2018

yes, and that's why I suggest using it, since NPM autogenerates that file, once you're starting a new project and want to lock dependencies yo do it an the package-lock gets updated and fixed too.
If we're not adding it to the project, neither adding it to the .gitignore we are going to handle this file manually, which is prone to errors.

@ghost
Copy link

ghost commented Jan 11, 2018

Fair enough.

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.

2 participants