Skip to content

pre-commit: add hook to trim trailing whitespace#8205

Merged
DaanHoogland merged 5 commits intoapache:mainfrom
jbampton:add-hook-trim-trailing-whitespace
May 28, 2024
Merged

pre-commit: add hook to trim trailing whitespace#8205
DaanHoogland merged 5 commits intoapache:mainfrom
jbampton:add-hook-trim-trailing-whitespace

Conversation

@jbampton
Copy link
Member

@jbampton jbampton commented Nov 9, 2023

https://github.com/pre-commit/pre-commit-hooks#trailing-whitespace

Description

This PR adds another hook or test to our pre-commit framework.

This hook automatically trims trailing whitespace when run locally.

Follow up PRs can target more file types if needed.

Extra unneeded whitespace is extra data. Save the planet :)

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

pre-commit run --all-files

or

pre-commit run trailing-whitespace --all-files

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 31.16%. Comparing base (4b4dfef) to head (9177024).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8205   +/-   ##
=========================================
  Coverage     31.15%   31.16%           
- Complexity    34034    34041    +7     
=========================================
  Files          5404     5404           
  Lines        380305   380305           
  Branches      55506    55506           
=========================================
+ Hits         118502   118524   +22     
+ Misses       246105   246077   -28     
- Partials      15698    15704    +6     
Flag Coverage Δ
simulator-marvin-tests 24.93% <ø> (+<0.01%) ⬆️
uitests 4.34% <ø> (ø)
unit-tests 16.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

looks generally good to me. Just to be clear and prevent confusion @jbampton , this is just a check and not a pre-commit corrective edit this hook is doing right?

@jbampton
Copy link
Member Author

jbampton commented Nov 9, 2023

Hey @DaanHoogland when the hook runs on GitHub with actions it is just a check that will either pass or fail.

But when pre-commit is setup locally and run this hook will also automatically trim trailing whitespace.

So yes it is a corrective edit. So all these whitespace clean ups were done automatically by the hook.

@yadvr yadvr added this to the 4.19.0.0 milestone Nov 27, 2023
@shwstppr shwstppr modified the milestones: 4.19.0.0, 4.19.1.0 Dec 14, 2023
@yadvr yadvr modified the milestones: 4.19.1.0, 4.20.0.0 Feb 5, 2024
@jbampton
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9015

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 15.28%. Comparing base (21af134) to head (6fd07be).

Additional details and impacted files
@@            Coverage Diff             @@
##               main    #8205    +/-   ##
==========================================
  Coverage     15.28%   15.28%            
- Complexity    11535    11536     +1     
==========================================
  Files          5425     5425            
  Lines        474138   474138            
  Branches      58984    59503   +519     
==========================================
+ Hits          72486    72492     +6     
+ Misses       393584   393578     -6     
  Partials       8068     8068            
Flag Coverage Δ
uitests 4.26% <ø> (ø)
unittests 16.02% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@jbampton
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9599

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian Build Failed (tid-10214)

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10245)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 49836 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8205-t10245-kvm-centos7.zip
Smoke tests completed. 130 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 414.61 test_events_resource.py

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland DaanHoogland merged commit 28e8e2d into apache:main May 28, 2024
@jbampton jbampton deleted the add-hook-trim-trailing-whitespace branch May 28, 2024 08:38
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants