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

Updating runner to use database versions and CI comparable to v7 #261

Merged
merged 18 commits into from
Feb 10, 2024

Conversation

WikiRik
Copy link
Member

@WikiRik WikiRik commented Aug 29, 2023

Fixes sequelize/sequelize#16434

This PR overhauls this repo once again.

  • CI is updated to use Node 10, 18, 20 (oldest v6, oldest v7, latest LTS). It started out as a copy of the CI on the core repo but the install-and-build artifact has been removed and prepare-ci/run SSCCE was added.
  • The setup folder is renamed to dev, to reflect the core repo. I also copied over the oldest/latest from there. We can update the oldest to the oldest supported v6 versions of the databases but I didn't think that was necessary as we only had one version before.
  • yarn.lock was added for the cache of GitHub Actions but I'm open to not using that feature and removing it again

In a future PR we should add DB2 support, this PR is already too large as it is.

And we should maybe consider deprecating this repo and updating SSCCE functionality in v6/main branches of the core repo and add a GitHub action that automatically tags a PR that (only) updates the SSCCE. That way we can easily filter out the SSCCE PRs and we don't have to maintain two separate versions.

@WikiRik WikiRik requested a review from ephys August 29, 2023 12:30
Copy link

@Hornwitser Hornwitser left a comment

Choose a reason for hiding this comment

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

Looks good, but there is still the issue that tests for v7 try to run on database versions that are unsupported. For example the postgres ones run on 9.5 and 10, but according to https://sequelize.org/releases/ only 11 and up is supported.

@WikiRik
Copy link
Member Author

WikiRik commented Aug 29, 2023

Looks good, but there is still the issue that tests for v7 try to run on database versions that are unsupported. For example the postgres ones run on 9.5 and 10, but according to sequelize.org/releases only 11 and up is supported.

Good point! We've updated files surrounding that in the sequelize repo for usage in the CI, but haven't updated that here. That shouldn't be why some of these tests are failing but I'll look into that later.

@WikiRik
Copy link
Member Author

WikiRik commented Dec 15, 2023

Update; I think it is indeed the postgres and MySQL versions that cause this to break. I'll need to update this with the files from the core repo.

@WikiRik WikiRik changed the title Update runner for v7 to run on Node 16 and up Updating runner to use database versions and CI comparable to v7 Feb 9, 2024
@WikiRik WikiRik removed the request for review from ephys February 9, 2024 09:52
@WikiRik WikiRik marked this pull request as ready for review February 9, 2024 09:52
Copy link

@Hornwitser Hornwitser left a comment

Choose a reason for hiding this comment

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

This is too involved with databases and CI tools I'm not familiar with for me to review.

@ephys
Copy link
Member

ephys commented Feb 10, 2024

Pretty great to see an update in this repository
We could merge this as-is, but I think we should consider splitting the "oldest" dialects into oldest-v7 and oldest-v6 (latest doesn't need to be split as both versions support the latest version of databases)

@WikiRik
Copy link
Member Author

WikiRik commented Feb 10, 2024

Good idea! I'll look into that for the next PR

@ephys ephys merged commit 1230359 into main Feb 10, 2024
45 checks passed
@ephys ephys deleted the WikiRik/Node-16-18 branch February 10, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants