sweetalert2 / sweetalert2-react-content

Official SweetAlert2 enhancer adding support for React elements as content
MIT License
702 stars 48 forks source link

Revert "chore: add yarn.lock" #86

Closed zenflow closed 5 years ago

zenflow commented 5 years ago

This reverts commit 0ccf37898130a7c4b0df4bc09bdc95f61375013a.

@limonte Please make PRs for non-trivial changes

This project still uses npm (with no lock file) in CI.

For consistent results, we should use just one package manager, and with one consistent choice for lock file vs no lock file. Now we don't have that. This PR fixes that.

I'm not sure what your aim was with the commit (?), but one note is that not having a lock file allows us to run the cron jobs (that you set up) that test the package with the latest in-range release of sweetalert2. Maybe what you wanted to do was pin all the other dependencies in package.json to exact versions?

limonte commented 5 years ago

This project still uses npm (with no lock file) in CI.

Should we switch to Yarn instead of removing lock file?

For consistent results, we should use just one package manager, and with one consistent choice for lock file vs no lock file. Now we don't have that. This PR fixes that.

Lock file, we have it in every sweetalert2/* repo, so let's be consistent on the org level. Lock files are useful (I guess no need to explain why) and they are the must in all programming languages I'm aware of. Except for JS of course because we're cool here. npm is terrible, it's understanding of lock file is ridiculous, but that's why there's Yarn to save us.

image

but one note is that not having a lock file allows us to run the cron jobs (that you set up) that test the package with the latest in-range release of sweetalert2.

That is the poor argument for not having lock file. Greenkeeper should responsible to run CI build on each sweetalert2 release. The benefit of using Greenkeeper instead of scheduled builds is that you'll know something is broken in a couple of minutes after a new sweetalert2 release, and with cron builds it'll be a couple of hours (up to 24 hours).

zenflow commented 5 years ago

That is the poor argument for not having lock file. Greenkeeper should responsible to run CI build on each sweetalert2 release. The benefit of using Greenkeeper instead of scheduled builds is that you'll know something is broken in a couple of minutes after a new sweetalert2 release, and with cron builds it'll be a couple of hours (up to 24 hours).

Yup, your logic makes sense. We have no use for npm, and we should switch to Yarn. In that case, I guess we have no reason for the cron builds? I'll disable them after we merge #87

limonte commented 5 years ago

Yup, your logic makes sense. We have no use for npm, and we should switch to Yarn. In that case, I guess we have no reason for the cron builds? I'll disable them after we merge #87

:+1:

limonte commented 5 years ago

:tada: This issue has been resolved in version 2.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: