-
Notifications
You must be signed in to change notification settings - Fork 4
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
Remove unused install deps scripts; clarify rules in CONTRIBUTING.md #35
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.
you need to work on creating better pr headers
the one you've used is very unclear
@ellipsis: Review this PR, please |
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 a2cf6d1 in 14 seconds
More details
- Looked at
194
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. .github/workflows/check_style.yml:61
- Draft comment:
RemovinghashFiles('**/install_deps_ubuntu.sh')
from the cache key might reduce its uniqueness, potentially leading to cache collisions. Consider adding another unique identifier if necessary. - Reason this comment was not posted:
Confidence changes required:50%
The PR removes the use ofhashFiles('**/install_deps_ubuntu.sh')
in the cache key, but the file is deleted in this PR, so this change is consistent. However, the cache key might become less unique, which could lead to cache collisions if other jobs use the same key format. It's important to ensure that the cache key remains unique enough to avoid such issues.
2. .github/workflows/release.yml:47
- Draft comment:
RemovinghashFiles('**/install_deps_ubuntu.sh')
from the cache key might reduce its uniqueness, potentially leading to cache collisions. Consider adding another unique identifier if necessary. - Reason this comment was not posted:
Confidence changes required:50%
The same issue with cache key uniqueness applies here as well. Removing the hash ofinstall_deps_ubuntu.sh
might lead to cache collisions if the key is not unique enough.
3. .github/workflows/test_conan.yml:43
- Draft comment:
RemovinghashFiles('**/install_deps_ubuntu.sh')
from the cache key might reduce its uniqueness, potentially leading to cache collisions. Consider adding another unique identifier if necessary. - Reason this comment was not posted:
Confidence changes required:50%
The same cache key uniqueness issue applies here as well. Removing the hash ofinstall_deps_ubuntu.sh
might lead to cache collisions if the key is not unique enough.
Workflow ID: wflow_KwVxxaZBrWf3vOpg
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
Remove unused dependency scripts and update CONTRIBUTING.md and workflow cache keys for clarity.
install_deps_macOS.sh
andinstall_deps_ubuntu.sh
fromscripts/
.check_style.yml
,release.yml
, andtest_conan.yml
to remove dependency oninstall_deps_ubuntu.sh
.CONTRIBUTING.md
regarding forking, rebasing, and commit message format.This description was created by
for a2cf6d1. It will automatically update as commits are pushed.