Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: Normalize repository, dropping node <10.13 support #11

Merged
merged 5 commits into from
Jan 31, 2022

Conversation

sttk
Copy link
Contributor

@sttk sttk commented Dec 11, 2021

This PR updates this project:

  • Updates dependency versions
  • Introduces GitHub Actions
  • Adds, changes and deletes some project files

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Just needs to be updated to use the main sver package instead of my fork with node 0.10 support.

LICENSE Outdated Show resolved Hide resolved
README.md Outdated
[gitter-url]: https://gitter.im/gulpjs/gulp
[gitter-image]: https://badges.gitter.im/gulpjs/gulp.svg
<!-- prettier-ignore-start -->
[range-support]: https://github.com/phated/sver-compat#range-support
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be updated to the official sver module. The documentation about "sver-compat" need to be updated to reference the "sver" project.

package.json Outdated
"test": "mocha --async-only",
"cover": "istanbul cover _mocha --report lcovonly",
"coveralls": "npm run cover && istanbul-coveralls"
"test": "nyc mocha --async-only"
},
"dependencies": {
"sver-compat": "^1.5.0"
Copy link
Member

Choose a reason for hiding this comment

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

This needs to use sver instead of my fork.

sttk and others added 2 commits January 15, 2022 14:45
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
@sttk
Copy link
Contributor Author

sttk commented Jan 15, 2022

@phated I've modified what you pointed out.

I replaced sver-compat to sver but a test about prerelease version caused an error. Therefore I changed the 3rd parameter of SemverRange.match from true to false, then the test was passed. I don't know whether this modification is correct.

@phated
Copy link
Member

phated commented Jan 31, 2022

Thank you for identifying that change and telling me. I've thought a lot about this and tested some things locally and I feel that our tests were testing our desired behavior—that a prerelease version is not covered by the final release, since there could be breaking changes.

However, there were bugs in sver-compat that weren't handling this correctly which I found while writing some tests against it. For example, 1.0.1-alpha.1 should also not be satisfied by ^1.0.0 because there might be changes that require a patch release in other packages to be compatible. I've added that test on this branch.

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this! 🙇

@phated phated merged commit 4856298 into gulpjs:master Jan 31, 2022
@github-actions github-actions bot mentioned this pull request Jan 31, 2022
@sttk sttk deleted the chore-normalize-repository branch February 6, 2022 11:39
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.

None yet

2 participants