-
Notifications
You must be signed in to change notification settings - Fork 72
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: enable mypy; actions cleanup #21
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to de1fe81 in 24 seconds
More details
- Looked at
132
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. .github/workflows/typecheck.yml:14
- Draft comment:
The versionv4
foractions/checkout
does not exist. Please usev3
, which is the latest version. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment addresses a potential issue with the specified version of 'actions/checkout'. If 'v4' does not exist, this would indeed require a code change to prevent the workflow from failing. This is a valid concern as it directly impacts the functionality of the workflow.
I might be assuming that the comment is correct without verifying the existence of 'v4'. It's possible that 'v4' does exist or is in development, and the comment could be outdated or incorrect.
I should verify the existence of 'v4' for 'actions/checkout' to ensure the comment is accurate. If 'v4' does not exist, the comment is valid and should be kept.
The comment should be kept if 'v4' for 'actions/checkout' does not exist, as it points out a necessary code change. Verification of the version's existence is needed.
2. .github/workflows/unit_tests.yml:15
- Draft comment:
The versionv4
foractions/checkout
does not exist. Please usev3
, which is the latest version. - Reason this comment was not posted:
Marked as duplicate.
3. .github/workflows/unit_tests.yml:17
- Draft comment:
The versionv5
foractions/setup-python
does not exist. Please usev4
, which is the latest version. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_l0rDM1eOKw9CaSsm
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 86ccf8b in 21 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_MNjZVT07UI6Xqpcn
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 2d9baa9 in 21 seconds
More details
- Looked at
10
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_s5Of0U3UpbUPN4B7
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on 4d3d9d8 in 18 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_RljMKgPlcXHwtjIe
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 99760c8 in 19 seconds
More details
- Looked at
32
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. .github/workflows/typecheck.yml:11
- Draft comment:
Theruns-on
value should beubuntu-latest
instead ofdepot-ubuntu-22.04
to ensure compatibility and avoid potential issues with a custom runner. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative as it assumes potential issues with a custom runner without evidence. The PR author might have specific reasons for using 'depot-ubuntu-22.04', such as specific dependencies or configurations. The comment does not point out a definite issue but rather a preference or a caution, which is not actionable.
I might be overlooking the importance of using 'ubuntu-latest' for broader compatibility. However, the PR author might have a valid reason for the change that the comment does not address.
The PR author likely has a specific reason for choosing 'depot-ubuntu-22.04', and without evidence of an issue, the comment is not actionable. The comment does not provide a clear code change requirement.
The comment should be removed as it is speculative and does not point out a definite issue with the code change.
Workflow ID: wflow_kAyj328ILrb1XRus
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 8ebc686 in 19 seconds
More details
- Looked at
16
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. .github/workflows/typecheck.yml:38
- Draft comment:
The cache key usessteps.setup-python.outputs.python-version
, which might not be set correctly. Consider usingpython-version
as per the setup-python action documentation. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is speculative and not directly related to the changes in the PR. It suggests a potential issue without certainty, which violates the rules for comments. The PR diff does not show any changes to the cache key or setup-python action, so the comment is not relevant to the current changes.
I might be missing the context of the entire workflow file, but based on the diff, the comment does not seem relevant. The comment could be addressing a potential issue that is not visible in the diff.
The comment should only address changes visible in the diff. Since the cache key was not changed in the PR, the comment is not relevant.
Remove the comment as it is speculative and not related to the changes in the PR diff.
Workflow ID: wflow_VnjXKDC1vUl9MV7H
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on d38f8c3 in 16 seconds
More details
- Looked at
14
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. .github/workflows/typecheck.yml:34
- Draft comment:
Theruns-on: depot-ubuntu-22.04
is non-standard. Consider usingubuntu-latest
or a specific version likeubuntu-20.04
for better compatibility. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is suggesting a change to a line that was not modified in the PR. According to the rules, comments should not be made on code that was not changed in the PR diff. Therefore, this comment should be removed.
I might be missing the context of why 'depot-ubuntu-22.04' is used, but since it wasn't changed in this PR, it's not relevant to comment on it.
Even if 'depot-ubuntu-22.04' is non-standard, the comment is not relevant to the current PR changes.
Remove the comment as it does not pertain to any changes made in this PR.
Workflow ID: wflow_sBV2p9bjdmJrQPJb
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary:
This PR adds MyPy type checking, updates GitHub Actions workflows, and modifies the Makefile for linting.
Key points:
.github/workflows/typecheck.yml
formain
branch.push
andpull_request
events.depot-ubuntu-22.04
forruns-on
in MyPy job..github/workflows/unit_tests.yml
to useactions/checkout@v4
andactions/setup-python@v5
.snok/install-poetry@v1
with specific settings.actions/cache@v4
.Makefile
to enable MyPy linting by uncommenting$(MYPY) .
in thelint
target.Generated with ❤️ by ellipsis.dev