Skip to content

ci(node): use Node 16 instead of 12#13703

Merged
sdepold merged 5 commits intosequelize:mainfrom
WikiRik:add-node-14-16-ci
Nov 28, 2021
Merged

ci(node): use Node 16 instead of 12#13703
sdepold merged 5 commits intosequelize:mainfrom
WikiRik:add-node-14-16-ci

Conversation

@WikiRik
Copy link
Copy Markdown
Member

@WikiRik WikiRik commented Nov 24, 2021

Pull Request Checklist

Please make sure to review and check all of these items:

  • Have you added new tests to prevent regressions? Not needed
  • Does npm run test or npm run test-DIALECT pass with this change (including linting)? No, but unrelated to this PR
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)? Not needed
  • Did you update the typescript typings accordingly (if applicable)? Not needed
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Did you follow the commit message conventions explained in CONTRIBUTING.md?

Description Of Change

Use Node 16 instead of Node 12 for tests, linting, test typing, and releasing.

@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 24, 2021

@sdepold I think this one can be merged even with the failing tests in main

@WikiRik WikiRik marked this pull request as ready for review November 24, 2021 20:41
@Keimeno
Copy link
Copy Markdown
Member

Keimeno commented Nov 24, 2021

I might even go the other way here personally. I doubt that running the tests on multiple node versions adds any significant value. Personally, I would only run the tests on the last supported node version v10.

This is also due to the problem that we have many tests randomly failing, this will only increase with this PR. This would require a maintainer to look through every failing workflow to check if it's a false negative, or if the failing workflow is due to the changed code

@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 24, 2021

I might even go the other way here personally. I doubt that running the tests on multiple node versions adds any significant value. Personally, I would only run the tests on the last supported node version v10.

This is also due to the problem that we have many tests randomly failing, this will only increase with this PR. This would require a maintainer to look through every failing workflow to check if it's a false negative, or if the failing workflow is due to the changed code

I see your point, but I'm just afraid of what will happen if something would be removed/changed between the last supported version and the current LTS. What we could do is only test on Node 10 and Node 16 and just hope they don't have changes reverted between those versions.

@WikiRik WikiRik added the status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. label Nov 24, 2021
@WikiRik WikiRik marked this pull request as draft November 24, 2021 20:52
@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 24, 2021

Marked it as draft again until we have reached an agreement on which Node versions we want to test on

@Keimeno
Copy link
Copy Markdown
Member

Keimeno commented Nov 28, 2021

I see your point, but I'm just afraid of what will happen if something would be removed/changed between the last supported version and the current LTS. What we could do is only test on Node 10 and Node 16 and just hope they don't have changes reverted between those versions.

This in my opinion is a good compromise. Especially, as I don't see them reverting breaking changes. We would still have numerous workflows running, but at least not over 60

@sdepold
Copy link
Copy Markdown
Member

sdepold commented Nov 28, 2021

I see your point, but I'm just afraid of what will happen if something would be removed/changed between the last supported version and the current LTS. What we could do is only test on Node 10 and Node 16 and just hope they don't have changes reverted between those versions.

This in my opinion is a good compromise. Especially, as I don't see them reverting breaking changes. We would still have numerous workflows running, but at least not over 60

sounds good to me

@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 28, 2021

Okay, then I will update this PR accordingly

@WikiRik WikiRik changed the title ci(node): add Node 14 and 16 to DB tests ci(node): use Node 16 instead of 12 Nov 28, 2021
@WikiRik WikiRik marked this pull request as ready for review November 28, 2021 13:15
@WikiRik WikiRik requested review from Keimeno and sdepold November 28, 2021 13:15
@WikiRik WikiRik removed the status: in discussion For issues and PRs. Community and maintainers are discussing the applicability of the issue. label Nov 28, 2021
@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 28, 2021

@sdepold can you update the required checks?

@sdepold
Copy link
Copy Markdown
Member

sdepold commented Nov 28, 2021

@sdepold can you update the required checks?

I think we talked about that the other day and i have to admit that I forgot the settings for that again. It's not sufficient as it is?

@WikiRik
Copy link
Copy Markdown
Member Author

WikiRik commented Nov 28, 2021

@sdepold can you update the required checks?

I think we talked about that the other day and i have to admit that I forgot the settings for that again. It's not sufficient as it is?

It's part of the Branch protection rules. It wants the results of the Node 12 tests as well right now, but it won't get them (as you can see it is still expecting 13 checks for this PR)

@sdepold sdepold merged commit 2d799b8 into sequelize:main Nov 28, 2021
@WikiRik WikiRik deleted the add-node-14-16-ci branch November 28, 2021 17:56
sdepold added a commit that referenced this pull request Dec 3, 2021
* feat(typescript): create alpha release with ts

* ci: add other ts versions to ci (#13686)

* fix: wrong interface used within mixin (#13685)

* fix(increment): fix key value broken query (#12985)

Co-authored-by: Sascha Depold <[email protected]>

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)

* fix(types): add instance member declaration (#13684)

* fix(types): add instance member declaration

* test(types): add static/instance members test cases

Co-authored-by: Sascha Depold <[email protected]>

* Create aws-lambda.md (#12642)

* fix(docs): add aws-lamda route (#13693)

* fix(types): allow override json function with custom return type (#13694)

* fix(types): allow override to json function with custom return type

* fix(types): remove automatic linter changes

Co-authored-by: sander-mol <[email protected]>

* ci(docs): add doc generation to checks (#13704)

* docs: add logo (#13700)

* docs: add logo

* fix(docs): logo not show up (#13699)

* fix(build): markdownlint

* docs(readme): use internal link

* docs(index.md): use internal link

* docs(index): update logo rendering in docs

* Center logo and headline

Co-authored-by: Sascha Depold <[email protected]>
Co-authored-by: Sascha Depold <[email protected]>

* test: fix mocha (#13707)

Co-authored-by: Rik Smale <[email protected]>
Co-authored-by: Sascha Depold <[email protected]>

* test: fix failing stack trace test (#13708)

* test: fix failing tests  (#13709)

* test: fix failing tests due to minification

Removes esbuild minification which was causing tests to fail and some changed behavior.

* Revert "fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)"

This reverts commit 4071378.

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13711)

* fix(upsert): fall back to DO NOTHING if no update key values provided (#13594)

* fix: remove erroneous .length in _.isEmpty

* refactor: use includes instead of or operators (#13706)

* docs(contributing): add section on adding/updating deps (#13715)

* docs(contributing): add Node versions

Fixes #13714

* docs(contributing): add section on adding/updating deps

* fix(types): add Col to where Ops (#13717)

* fix(types): add Col to where Ops

* fix(types): tests

* fix(data-types): unnecessary warning when getting data with DATE dataTypes (#13712)

* fix(data-types): unnecessary error when getting data with DATE dataTypes

* fix(data-types): date stringify mariadb

* fix(types): add missing schema field to sequelize options

Fixes #12606

Co-authored-by: Sascha Depold <[email protected]>

* fix(example): fix coordinates format as per GeoJson (#13718)

* fix(example): fix coordinates format as per GeoJson

* Update data-types.js

Co-authored-by: Sascha Depold <[email protected]>

* ci(node): use Node 16 instead of 12 (#13703)

* ci(node): add Node 14 and 16 to DB tests

* ci(node): move linting, test typing and release to Node 16

* ci(docs): use Node 16 for docs

* ci(node): run tests on Node 10 and 16

Co-authored-by: Rik Smale <[email protected]>

* refactor(postgres): move `clientMinMessages` from general to pg options (#13720)

* refactor(postgres): move `clientMinMessages` from general to pg options

* refactor(postgres): address review comments

* refactor(postgres): address review comments

* refactor(postgres): fix pipeline

* fix(dialect): try to fix flaky test

* fix(dialect): try to fix flaky test

Co-authored-by: Jesse Peng <[email protected]>

* refactor(build): use rm instead of rmdir for Node 14 and up (#13702)

* fix(build): refactor rmdir to rm

* refactor(build): only use rm in Node 14 and up

* refactor(build): move consts inside function

* refactor(build): fix definition

Co-authored-by: Rik Smale <[email protected]>
Co-authored-by: Sascha Depold <[email protected]>

* docs(postgres): warn about deprecated clientMinMessage option (#13727)

* docs(postgres): warn about deprecated clientMinMessage option

* docs(deprecation-warning): fix typo in deprecation warning

* docs(deprecation-warning): fix typo in deprecation warning

* meta(deps): upgrade (dev)deps (#13729)

Co-authored-by: Rik Smale <[email protected]>

* test(integration): mark and fix flaky test (#13735)

* feat(dialect): snowflake dialect support (#13406)

Co-authored-by: Mohamed El Mahallawy <[email protected]>
Co-authored-by: bparan <[email protected]>
Co-authored-by: Matthew Blasius <[email protected]>
Co-authored-by: WeRDyin <[email protected]>
Co-authored-by: Marco Gonzalez <[email protected]>
Co-authored-by: Constantin Metz <[email protected]>
Co-authored-by: Sander Mol <[email protected]>
Co-authored-by: sander-mol <[email protected]>
Co-authored-by: Rik Smale <[email protected]>
Co-authored-by: Fauzan <[email protected]>
Co-authored-by: Rik Smale <[email protected]>
Co-authored-by: AllAwesome497 <[email protected]>
Co-authored-by: manish kakoti <[email protected]>
Co-authored-by: Marco Kerwitz <[email protected]>
Co-authored-by: Jesse Peng <[email protected]>
Co-authored-by: Jesse Peng <[email protected]>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 4, 2021

🎉 This PR is included in version 6.12.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 6.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

aliatsis pushed a commit to creditiq/sequelize that referenced this pull request Jun 2, 2022
* ci(node): add Node 14 and 16 to DB tests

* ci(node): move linting, test typing and release to Node 16

* ci(docs): use Node 16 for docs

* ci(node): run tests on Node 10 and 16

Co-authored-by: Rik Smale <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants