Skip to content
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

Execute prettier #4291

Merged
merged 3 commits into from
Jan 4, 2025
Merged

Execute prettier #4291

merged 3 commits into from
Jan 4, 2025

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Jan 2, 2025

  • run prettier once on the code base to minimize code changes in future PR's
  • fix most of remaining sonar cloud issues in 2nd commit
  • adds git blame ignore file for first commit

@haslinghuis haslinghuis added this to the 11.0 milestone Jan 2, 2025
@haslinghuis haslinghuis self-assigned this Jan 2, 2025
Copy link

netlify bot commented Jan 2, 2025

Deploy Preview for origin-betaflight-app ready!

Name Link
🔨 Latest commit ec608a2
🔍 Latest deploy log https://app.netlify.com/sites/origin-betaflight-app/deploys/67799948c31dd3000845fdfb
😎 Deploy Preview https://deploy-preview-4291.dev.app.betaflight.com
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

i'm not testing it 🤣

@haslinghuis haslinghuis force-pushed the run-prettier branch 4 times, most recently from b7a9cc2 to 326589a Compare January 2, 2025 21:42
@VitroidFPV
Copy link
Member

VitroidFPV commented Jan 2, 2025

I like the idea of removing it from the git blame, similar to what would happen when AM32 gets re-formatted am32-firmware/AM32#138

@blckmn
Copy link
Member

blckmn commented Jan 3, 2025

I'm on the fence with doing it in bulk. What are the downsides? Causes rebasing requirement of existing PRs? Testing?

What are the downsides to doing it as files are touched?

@haslinghuis
Copy link
Member Author

@blckmn added 3 lines in 6fbeb83 - the rest was prettier doing it's job.

  • Testing: no functional changes are made.
  • Existing PR's: will need a rebase.
  • Touch: Doing it not once will make review harder for some time.

@blckmn
Copy link
Member

blckmn commented Jan 3, 2025

@haslinghuis with the volume of changes to every file rebasing will be a pain. I'm not sure it will make reviewing files much harder...

@haslinghuis
Copy link
Member Author

@blckmn - an example how huge simple PR would be: #4294

Copy link

sonarqubecloud bot commented Jan 4, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@blckmn
Copy link
Member

blckmn commented Jan 4, 2025

@blckmn - an example how huge simple PR would be: #4294

There's no real good way to do such a massive change without it being a complete pain one way or another.

@haslinghuis haslinghuis merged commit 7614ad3 into betaflight:master Jan 4, 2025
8 of 9 checks passed
@haslinghuis haslinghuis deleted the run-prettier branch January 4, 2025 21:43
@nerdCopter
Copy link
Member

nerdCopter commented Jan 5, 2025

post-merge: flashed whoop, pasted diff, went to each tab pressed save; compared resultant diff to original = no change.

great option to git-blame ignore 😃

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

Successfully merging this pull request may close these issues.

4 participants