Skip to content

Conversation

CarviFPV
Copy link

@CarviFPV CarviFPV commented Sep 12, 2025

Removed the word "Roll" from the Pitch Sliders wich had confusing some Users. Issue: #4574

Summary by CodeRabbit

  • Bug Fixes
    • Corrected Pitch-related labels to remove “Roll” and accurately reflect Pitch-only ratios on damping and tracking controls.
  • Documentation
    • Updated PID tuning help text to match revised Pitch labels and improve wording, punctuation, and capitalization.
  • Style
    • Fixed typos ("Enchances" → "Enhances", "strenght" → "strength") and standardized phrasing (e.g., "Raises or lowers") for clearer UI copy.

Copy link
Contributor

coderabbitai bot commented Sep 12, 2025

Walkthrough

Updated three English localization entries in locales/en/messages.json: adjusted pitch-related labels to "Pitch: D" and "Pitch: P, I & FF", corrected punctuation/typos and capitalization in the PID sliders help text, and standardized some HTML line-break formatting.

Changes

Cohort / File(s) Change summary
Localization updates
locales/en/messages.json
Modified messages: pidTuningRollPitchRatioSlider ("Pitch:Roll D" → "Pitch: D"), pidTuningPitchPIGainSlider ("Pitch:Roll P, I & FF" → "Pitch: P, I & FF"); updated pidTuningPidSlidersHelp text for corrected labels, grammar/typos ("Enchances"→"Enhances", "strenght"→"strength"), punctuation, and standardized <br /> usage.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 264c55f and 37b14b3.

📒 Files selected for processing (1)
  • locales/en/messages.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • locales/en/messages.json

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The current PR description is minimal and does not follow the repository's required template: it only states the intent and references Issue #4574 but omits required sections such as background, a summary of changed files/strings, testing or verification steps, and CI/build expectations, and it contains typographical errors ("wich"). Because these template sections and key details are missing, the description does not meet the repository's standards and is insufficient for review. Please update the PR description using the repository template: add a brief background and rationale, list the files and exact string changes, include testing or verification steps (how you validated the strings and any UI checks), confirm branch/source details (not from master) and reference Issue #4574, and fix typos and grammar. Also mention expected CI checks or whether CI has already passed and any reviewer notes to expedite merging.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely identifies the primary change — removal of the word "Roll" from Pitch sliders — and matches the PR objectives and file summaries showing only localization string edits. It is a short, single sentence that a reviewer scanning history can understand. For consistency with other PRs you may opt for present-tense phrasing (e.g., "Remove 'Roll' from pitch slider labels"), but no change is required.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
locales/en/messages.json (1)

4538-4539: Standardize line break tag.

Use <br /> (self-closing) to match the rest of the file and avoid mixed markup.

Apply this diff:

-        "message": "Pitch Tracking:<br><i><small>Pitch: P, I & FF</small></i>",
+        "message": "Pitch Tracking:<br /><i><small>Pitch: P, I & FF</small></i>",
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25def4c and 264c55f.

📒 Files selected for processing (1)
  • locales/en/messages.json (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T11:37:09.758Z
Learnt from: haslinghuis
PR: betaflight/betaflight-configurator#4490
File: locales/en/messages.json:1534-1545
Timestamp: 2025-05-26T11:37:09.758Z
Learning: In the Betaflight Configurator repository, only English messages should be added directly to `locales/en/messages.json`; other language translations are handled via Crowdin automation and should not be manually updated in PRs.

Applied to files:

  • locales/en/messages.json
🔇 Additional comments (1)
locales/en/messages.json (1)

4530-4531: ```shell
#!/bin/bash
set -euo pipefail

echo '== rg --debug: pattern Pitch[:\s-]*Roll =='
rg --debug -nI --hidden -S -P -C3 'Pitch[:\s-]*Roll' || true

echo '== rg (no debug): Pitch[:\s-]*Roll =='
rg -nI --hidden -S -P -C3 'Pitch[:\s-]*Roll' || true

echo '== rg variants (Pitch.*Roll, exact etc.) =='
rg -nI --hidden -S -P -C3 'Pitch.*Roll' || true
rg -nI --hidden -S -P -C3 'Pitch:Roll' || true
rg -nI --hidden -S -P -C3 'Pitch-Roll' || true
rg -nI --hidden -S -P -C3 'Pitch Roll' || true

echo '== search for "Pitch" occurrences (context) =='
rg -nI --hidden -S -C3 'Pitch' || true

echo '== find messages.json files and search within them =='
find . -type f -name 'messages.json' -print0 | xargs -0 -r rg -n --hidden -S -P -C3 'Pitch[:\s-]*Roll' || true

echo '== show locales/en/messages.json around lines 4520-4540 =='
if [ -f locales/en/messages.json ]; then
sed -n '4520,4540p' locales/en/messages.json || true
else
echo 'locales/en/messages.json not found'
fi

echo '== done =='


</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@haslinghuis haslinghuis moved this to App in 2025.12.0 Sep 12, 2025
@haslinghuis haslinghuis added this to the 2025.12 milestone Sep 12, 2025
@haslinghuis haslinghuis removed this from 2025.12.0 Sep 12, 2025
@haslinghuis haslinghuis removed this from the 2025.12 milestone Sep 12, 2025
@haslinghuis
Copy link
Member

This is intentional to signify that it is a multiplier over the roll values.

@haslinghuis haslinghuis reopened this Sep 12, 2025
@haslinghuis
Copy link
Member

haslinghuis commented Sep 12, 2025

Perhaps we keep the grammatical fixes

Copy link

Copy link
Contributor

@Canaill51
Copy link

Preview URL: https://37b14b31.betaflight-configurator.pages.dev

@haslinghuis @CarviFPV , I checked your changes (in english version), this seems to be good, thanks!
How to update all language GUI?

@haslinghuis
Copy link
Member

@Canaill51 Keeping Roll here is intentional to signify it is a multiplier over the roll values.

@haslinghuis
Copy link
Member

@limonspb - what do you think ?

@nerdCopter
Copy link
Member

This is intentional to signify that it is a multiplier over the roll values.

i believe this is true. : colon typically indicates a ratio -- i.e. roll to pitch ratio

@haslinghuis
Copy link
Member

@CarviFPV do you care to update the tooltip instead to point out the ratio here ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants