typeorm icon indicating copy to clipboard operation
typeorm copied to clipboard
verified publisher icon typeorm

Lint on GitHub actions

Open stim371 opened this issue 1 year ago • 3 comments

Description of change

Add linting step to Github Actions test pipeline.

Pull-Request Checklist

  • [x] Code is up-to-date with the master branch
  • [x] npm run format to apply prettier formatting
  • [ ] npm run test passes with this change
  • [ ] This pull request links relevant issues as Fixes #0000
  • [ ] There are new or updated unit tests validating the change
  • [ ] Documentation has been updated to reflect this change
  • [ ] The new commits follow conventions explained in CONTRIBUTING.md

stim371 avatar Dec 11 '24 07:12 stim371

This looks good. One thing I'm wondering is whether we can move the node version matrix up to the top level to avoid repeating at every job. This will make it less error-prone when we start to enable more versions.

michaelbromley avatar Dec 11 '24 08:12 michaelbromley

This looks good. One thing I'm wondering is whether we can move the node version matrix up to the top level to avoid repeating at every job. This will make it less error-prone when we start to enable more versions.

Here's an updated version with the matrix pulled up a level. It requires there to be two sub-files, but it's kind of nice to see that there are two different setups happening.

stim371 avatar Dec 12 '24 08:12 stim371

I would rename database-tests-oracle-cockroachdb.yml to something like database-compose-tests.yml since it uses docker compose.

OSA413 avatar Dec 12 '24 11:12 OSA413

Wouldn't it be a good idea to define a matrix with custom parameters for each driver type? I think it will be much more concise.

Nested structures such as services which differ across per-driver workflows it seems like could be done without too much effort

https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow#expanding-or-adding-matrix-configurations

@nikelborm can you provide a short example?

This PR was originally just for adding linting. We could continue to refactor and improve in follow-on ones further.

stim371 avatar Dec 14 '24 00:12 stim371

True, I guess we are mixing concerns here a bit. And now it caused a merge conflict 😅

I suggest let's just get the conflict resolved, merge the linting, and then include any other workflows optimizations in a separate PR

michaelbromley avatar Dec 14 '24 09:12 michaelbromley