fix: change sloglog to INFO #2926
Conversation
WalkthroughThe changes in the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/pika.yml (1)
Line range hint
1-284: Consider harmonizing build environments across jobsWhile the changes to the macOS build job are generally good, they introduce some inconsistencies with the other build jobs (Ubuntu and Rocky Linux). To ensure consistent behavior across all environments, consider the following:
Align Python versions: The macOS job now uses Python 3.13 (which we've suggested changing to 3.12), while other jobs might be using different versions. Consider specifying the same Python version across all jobs.
Compiler versions: The macOS job explicitly uses gcc-10, while other jobs might be using different compiler versions. If possible, try to use consistent compiler versions across all environments.
Build flags and options: Ensure that build flags and options (like
-DUSE_PIKA_TOOLS=ONand-DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address) are consistent across all jobs where applicable.To improve maintainability and consistency, consider creating a shared configuration file or using GitHub Actions' composite actions to define common setup steps and configurations that can be reused across different jobs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/pika.yml (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
.github/workflows/pika.yml (2)
Line range hint
182-184: Explicit C compiler setting looks goodThe explicit setting of the C compiler to gcc-10 is a good practice for ensuring build consistency. This change aligns well with the installation of gcc@10 in the previous step.
For improved clarity and consistency, consider moving the
CCenvironment variable setting to the same line as thecmakecommand:- export CC=/usr/local/opt/gcc@10/bin/gcc-10 - cmake -B build -DCMAKE_C_COMPILER=/usr/local/opt/gcc@10/bin/gcc-10 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache + cmake -B build -DCMAKE_C_COMPILER=/usr/local/opt/gcc@10/bin/gcc-10 -DCMAKE_CXX_COMPILER=/usr/local/opt/gcc@10/bin/g++-10 -DUSE_PIKA_TOOLS=ON -DCMAKE_BUILD_TYPE=${{ env.BUILD_TYPE }} -DCMAKE_CXX_FLAGS_DEBUG=-fsanitize=address -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccacheThis change removes the redundant
exportand ensures that both C and C++ compilers are explicitly set.
177-177:⚠️ Potential issueCaution: Python 3.13 is not yet released and
--overwriteflag usageThe update to Python 3.13 is premature as this version is not yet released (as of October 2024). This could lead to compatibility issues or failed builds. Consider reverting to a stable version like 3.11 or 3.12.
Additionally, the
--overwriteflag has been added to the brew install command. While this can resolve conflicts, it may potentially overwrite important files. Use this flag with caution and ensure it's necessary for your build process.Consider applying this change:
- brew install --overwrite [email protected] autoconf protobuf llvm wget git + brew install [email protected] autoconf protobuf llvm wget gitTo check the latest stable Python version available in Homebrew, you can run:
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/pika.yml (2 hunks)
🧰 Additional context used
🪛 actionlint
.github/workflows/pika.yml
159-159: label "macos-11" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (3)
.github/workflows/pika.yml (3)
Line range hint
180-183: LGTM: Explicit compiler configurationGood improvement on explicitly setting the C compiler path. This makes the build more deterministic and helps prevent compiler mismatches.
Line range hint
1-1: PR objective and changes mismatchThe PR description mentions changing slow log levels from ERROR to INFO, but the actual changes are modifying the macOS CI configuration. This suggests either:
- The PR description is incorrect
- The log level changes are missing
- Wrong files were committed
Please clarify the intended changes and ensure the PR description matches the actual modifications.
🧰 Tools
🪛 actionlint
159-159: label "macos-11" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
177-178: Verify Python dependency requirementsThe removal of
[email protected]from the brew install command might impact build requirements. Please verify that Python is not needed for:
- Build scripts
- Test execution
- Development tools
.github/workflows/pika.yml
Outdated
| @@ -169,12 +169,12 @@ jobs: | |||
| - name: ccache | |||
| uses: hendrikmuhs/[email protected] | |||
| with: | |||
| key: macos-12 | |||
| key: macos-11 | |||
There was a problem hiding this comment.
Update cache key to match supported runner version
The cache key should be updated to match the runner version for consistency.
- key: macos-11
+ key: macos-12📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key: macos-11 | |
| key: macos-12 |
.github/workflows/pika.yml
Outdated
| @@ -156,7 +156,7 @@ | |||
| sh integrate_test.sh | |||
| build_on_macos: | |||
| runs-on: macos-12 | |||
| runs-on: macos-11 | |||
There was a problem hiding this comment.
Warning: Unsupported macOS runner version
The macos-11 runner label is not in the list of supported GitHub-hosted runners. Consider using one of the following supported versions:
macos-latestmacos-13macos-12
- runs-on: macos-11
+ runs-on: macos-12📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runs-on: macos-11 | |
| runs-on: macos-12 |
🧰 Tools
🪛 actionlint
159-159: label "macos-11" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-14.0", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-13.0", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "macos-12.0", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
将慢日志的日志级别从 ERROR 日志调整为 INFO 日志
Summary by CodeRabbit