Fix Type issues in main branch #832
Conversation
sscargal
left a comment
There was a problem hiding this comment.
The remaining changes look good to me.
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Ensure uv.lock is correct |
There was a problem hiding this comment.
This is a good idea.
FYI, we have .github/workflows/check-uvlockfile.yml that runs uv lock --check. It is/was designed to highlight out-of-sync uv.lock files at PR time and have the developer update their file.
This change would ensure it's always synchronized, making the check-uvfile redundant, which is a good thing. However, it must be the first workflow to run to avoid issues with other workflows that require the uvlock to be in sync. Alternatively, we create a dedicated uv workflow to sync the file and ensure it's the first to run.
I'm open to suggestions.
There was a problem hiding this comment.
We can remove check-uvlockfile or maybe have it run always. I'll check if it would have caught this.
With the way are workflows are currently set up (all independent) there isn't a easy way to create that type of dag flow.
I think in general it is ok to have a single lint check validate the uv, assuming people only merge when all tests are successful.
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Ensure uv.lock is correct | ||
| run: uv lock --refresh && git diff --exit-code |
There was a problem hiding this comment.
How is this supposed to work?
Why is it the --refresh option?
There was a problem hiding this comment.
--refresh refreshes the cache before running uv lock.
With the git diff returning an error code if there are any changes.
* Fix Type issues in main branch * Rename to generic lint workflow
Regression was introduced into main due to a dependency missing, causing ty to not be able to validate types.
Add a lint check to validate
uv.lockand fix main issues.