-
-
Notifications
You must be signed in to change notification settings - Fork 543
chore: Release - rc → master #1489
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughChanges refactor CPU and GPU info display from hardcoded color logic to CSS class-based styling. Scripts now emit JSON with computed temperature and utilization class names instead of color values. New CSS files define colors and font-weights for temperature and utilization buckets. Wallpaper script adds branching logic for HYPRLAND detection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 6
🧹 Nitpick comments (5)
Configs/.local/lib/hyde/wallpaper.hyprpaper.sh (1)
29-33: Add error handling for service management operations.The service management logic lacks error handling. Consider adding checks and user notifications for failures:
- The
systemctl --user restartcommand could fail- The
app2unit.shscript might not exist or could encounter errors- Users won't be notified if wallpaper setup fails
🔎 Proposed enhancement with error handling
if systemctl --user is-active --quiet hyprpaper.service; then - systemctl --user restart hyprpaper.service + if ! systemctl --user restart hyprpaper.service; then + print_log -sec "wallpaper" -stat "error" "Failed to restart hyprpaper.service" + exit 1 + fi else - app2unit.sh -- hyprpaper --config "$XDG_STATE_HOME/hyde/hyprpaper.conf" + if ! command -v app2unit.sh &>/dev/null; then + print_log -sec "wallpaper" -stat "error" "app2unit.sh not found" + exit 1 + fi + if ! app2unit.sh -- hyprpaper --config "$XDG_STATE_HOME/hyde/hyprpaper.conf"; then + print_log -sec "wallpaper" -stat "error" "Failed to create hyprpaper unit" + exit 1 + fi fiConfigs/.local/share/waybar/styles/classes/cpuinfo.css (1)
13-15: Consider reordering selectors for readability.The selectors are listed as
temp-25, temp-35, temp-30instead of sequential order. While functionally correct, reordering totemp-25, temp-30, temp-35would improve readability.🔎 Suggested reordering
-#custom-cpuinfo.temp-25, -#custom-cpuinfo.temp-35, -#custom-cpuinfo.temp-30 { +#custom-cpuinfo.temp-25, +#custom-cpuinfo.temp-30, +#custom-cpuinfo.temp-35 { color: #87ceeb; }Configs/.local/share/waybar/styles/classes/gpuinfo.css (1)
13-15: Consider reordering selectors for readability.The selectors are listed as
temp-25, temp-35, temp-30instead of sequential order. While functionally correct, reordering totemp-25, temp-30, temp-35would improve readability and consistency with expected numerical ordering.🔎 Suggested reordering
-#custom-gpuinfo.temp-25, -#custom-gpuinfo.temp-35, -#custom-gpuinfo.temp-30 { +#custom-gpuinfo.temp-25, +#custom-gpuinfo.temp-30, +#custom-gpuinfo.temp-35 { color: #87ceeb; }Configs/.local/lib/hyde/gpuinfo.sh (2)
152-167: LGTM! CSS class computation logic is correct.The temperature and utilization bucketing logic correctly computes CSS classes for styling:
- Temperature: 5°C buckets (temp-0, temp-5, ..., temp-995)
- Utilization: 10% buckets (util-0, util-10, ..., util-100)
- Bounds checking prevents invalid values
Optional: Add braces for consistency
For consistency with other variable expansions in this block, consider adding braces:
- local util_val=${utilization%.*} + local util_val=${utilization%%.*}Note: Also changed
%.*to%%.*to handle edge cases where multiple dots might exist (though unlikely in this context).
168-189: Manual JSON construction is acceptable but consider usingjqfor safety.The JSON output now includes CSS classes, percentage, and alt fields as intended. While manual string concatenation works here since values are controlled, using
jqwould provide better safety against special characters.Optional: Consider using jq for JSON construction
For more robust JSON construction, consider using
jq:jq -nc \ --arg text "$thermo $temperature°C" \ --arg tooltip "$full_tooltip_text" \ --arg temp_class "$temp_class" \ --arg util_class "$util_class" \ --argjson percentage "$temp_pct" \ --arg alt "$temp_bucket" \ '{text: $text, tooltip: $tooltip, class: [$temp_class, $util_class], percentage: $percentage, alt: $alt}'This eliminates potential issues with special characters in values.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.mdConfigs/.local/lib/hyde/cpuinfo.shConfigs/.local/lib/hyde/gpuinfo.shConfigs/.local/lib/hyde/screenshot.shConfigs/.local/lib/hyde/wallpaper.hyprpaper.shConfigs/.local/share/waybar/styles/classes/cpuinfo.cssConfigs/.local/share/waybar/styles/classes/gpuinfo.cssConfigs/.local/share/waybar/styles/defaults.css
🧰 Additional context used
🧬 Code graph analysis (1)
Configs/.local/lib/hyde/screenshot.sh (2)
Configs/.local/lib/hyde/fastfetch.sh (1)
USAGE(7-18)Configs/.local/lib/hyde/screenrecord.sh (1)
USAGE(11-42)
🪛 Shellcheck (0.11.0)
Configs/.local/lib/hyde/gpuinfo.sh
[warning] 219-219: cpuinfo_file is referenced but not assigned (did you mean 'gpuinfo_file'?).
(SC2154)
🔇 Additional comments (5)
Configs/.local/lib/hyde/wallpaper.hyprpaper.sh (1)
17-17: LGTM! Good refactor to use the variable.Using
${cached_thumb}instead of hardcoding the path improves maintainability and consistency.CHANGELOG.md (1)
9-16: LGTM! Clear documentation of the CSS class-based styling change.The changelog entry accurately describes the shift from hardcoded colors to CSS classes for CPU and GPU info, which aligns with the code changes in this PR.
Configs/.local/lib/hyde/screenshot.sh (1)
8-8: LGTM! Cosmetic formatting improvements.These changes improve code consistency without altering functionality:
- Heredoc delimiter formatting standardized
- Whitespace normalized around redirects
- Case statement formatting cleaned up
Also applies to: 55-55, 117-123
Configs/.local/lib/hyde/cpuinfo.sh (1)
65-94: LGTM! Robust Perl-based JSON parsing implementation.The Perl script correctly:
- Implements a fallback chain for JSON parsers (Cpanel::JSON::XS → JSON::XS → JSON::PP)
- Extracts temperature data from multiple chip types (coretemp, k10temp, zenpower)
- Handles missing data gracefully with fallback logic on lines 96-102
Configs/.local/share/waybar/styles/defaults.css (1)
15-18: LGTM! The new CSS imports follow the existing pattern and the referenced files exist in the repository.
* Modify update instructions Replaced git pull with git fetch --depth 1 and git reset --hard to maintain a shallow clone * docs: Update README.es.md and fix translations updated the Spanish README to follow the same format as the README, translated the new content, and corrected some translations that sounded odd. * docs: Forgot to translate an animated subtitle * docs: Small correction * docs: Fix broken assets * docs: Fix incorrect path * docs: Resolve bot's nitpicks * docs: Add warning about uncommited changes * docs: Add git fetch safeguard --------- Co-authored-by: NeatTaken <137969671+NeatTaken@users.noreply.github.com> Co-authored-by: Khing <53417443+kRHYME7@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This is an automated PR to promote changes from
rctomaster.Please review and test before merging.
See TESTING.md for complete testing instructions.
According to our release policy, this PR is expected to be merged on: 1st or 3rd Friday of the month
Testers are encouraged to test the changes before merging.
Please note that this schedule may be adjusted based on the needs of the project.
Please review the changes carefully before merging.