-
Notifications
You must be signed in to change notification settings - Fork 7
Remove armv7 support #7
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
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.
👍 Looks good to me! Reviewed everything up to be378cc in 1 minute and 46 seconds
More details
- Looked at
33
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. scripts/generate-formulas/main.go:166
- Draft comment:
Removed the LinuxArmv7 checksum assignment. Ensure that related documentation and asset generation scripts are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that documentation and scripts are updated, which violates the rule against asking the author to ensure things are tested or updated. It doesn't provide a specific suggestion or ask for confirmation of an intentional change.
2. scripts/generate-formulas/main_test.go:26
- Draft comment:
Removed usage of LinuxArmv7 from test data. Verify that tests overall still cover all supported architectures. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that tests still cover all supported architectures after removing LinuxArmv7 from test data. This falls under asking the author to ensure the change is tested, which is against the rules.
3. scripts/generate-formulas/main.go:25
- Draft comment:
Removed LinuxArmv7 support. Ensure armv7 assets are now intentionally ignored. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. scripts/generate-formulas/main_test.go:28
- Draft comment:
Removed LinuxArmv7 value from test struct to align with supported architectures. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, explaining why a value was removed. It doesn't provide a suggestion, ask for confirmation, or point out a potential issue.
Workflow ID: wflow_HKGJ3LVjsUVDqFgZ
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 e6b5419 in 1 minute and 1 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. .github/workflows/tests.yml:19
- Draft comment:
Confirm that cache key behavior remains as expected with actions/cache@v4 upgrade. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. .github/workflows/tests.yml:19
- Draft comment:
Ensure actions/cache@v4 is stable. If v4 isn't officially recommended or has breaking changes, consider pinning a specific commit hash or use the documented release. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is about ensuring the stability of a specific version of a dependency (actions/cache@v4
). It suggests pinning a specific commit hash or using a documented release ifv4
isn't stable. This is a dependency-related comment, which violates the rule against commenting on dependencies.
Workflow ID: wflow_H1DHmLQH2F71FQsH
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.
LGTM
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 fc6435a in 50 seconds
More details
- Looked at
19
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. .github/workflows/publish.yml:21
- Draft comment:
Good fix: quoting variables in the brew command prevents word splitting issues. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. .github/workflows/publish.yml:33
- Draft comment:
Quoting the branch variable improves safety with branch names containing spaces. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. .github/workflows/publish.yml:21
- Draft comment:
Good improvement: quoting the variables in the brew pr-pull command avoids word splitting issues if the values contain spaces. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. .github/workflows/publish.yml:33
- Draft comment:
Quoting the BRANCH variable in the git push command ensures safe expansion and prevents potential issues with spaces or special characters. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_v2JZZkrdSqWNDCHw
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.
LGTM
Important
Remove
armv7
support frommain.go
,main_test.go
, and update workflow files for consistency and newer action versions.LinuxArmv7
fromShas
struct inmain.go
and related logic inparseChecksums()
.TestValidateNoPanic
inmain_test.go
to removeLinuxArmv7
test case.publish.yml
to use double quotes for variables inbrew pr-pull
andgit push
commands.tests.yml
to useactions/cache@v4
instead ofv1
.This description was created by
for fc6435a. It will automatically update as commits are pushed.