Add `pre-commit` workflow with 3 Git hooks by jbampton · Pull Request #6273 · apache/cloudstack

jbampton · 2022-04-16T13:37:07Z

Use pre-commit to autofix the mixed line endings

"Git hook scripts are useful for identifying simple issues before submission to code review. We run our hooks on every commit to automatically point out issues in code such as missing semicolons, trailing whitespace, and debug statements. By pointing these issues out before code review, this allows a code reviewer to focus on the architecture of a change while not wasting time with trivial style nitpicks."

https://pre-commit.com/

Description

This PR adds pre-commit as a GitHub Action. There are many Git hooks you can add but I just added 3 for easy review and I can expand on this framework in other PRs going forwards.

Apache Airflow has an amazing pre-commit framework almost seen as best practice.

https://github.com/apache/airflow/blob/main/.pre-commit-config.yaml

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

github-actions · 2022-07-01T13:59:51Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

github-actions · 2022-07-07T07:48:33Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

DaanHoogland · 2022-07-07T08:42:24Z

@jbampton there seem to be a lot of irrelevant changes in this PR. Maybe line \n vs. \r? Are all of these necessary?

shwstppr · 2022-07-29T06:53:03Z

Changing milestone to 4.18.0 as this targeted for main

jbampton · 2022-08-04T22:50:13Z

Hey @DaanHoogland best practice is to use only one type of line ending in a project.

From the prettier site:

"When people collaborate on a project from different operating systems, it becomes easy to end up with mixed line endings in a shared git repository. It is also possible for Windows users to accidentally change line endings in a previously committed file from LF to CRLF. Doing so produces a large git diff and thus makes the line-by-line history for a file (git blame) harder to explore."

https://prettier.io/docs/en/options.html#end-of-line

DaanHoogland · 2022-11-10T13:21:11Z

@blueorangutan package

blueorangutan · 2022-11-10T13:22:03Z

@DaanHoogland a 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 · 2022-11-10T14:15:48Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. LL-JID 246

DaanHoogland · 2022-11-13T09:58:58Z

@blueorangutan test

blueorangutan · 2022-11-13T10:00:02Z

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

blueorangutan · 2022-11-13T10:24:14Z

Trillian Build Failed (tid-69)

blueorangutan · 2022-11-13T23:10:26Z

Trillian test result (tid-74)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43466 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t74-kvm-centos7.zip
Smoke tests completed. 101 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	`Failure`	476.64	test_kubernetes_clusters.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	575.26	test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers	`Failure`	404.86	test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn	`Failure`	754.90	test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options	`Error`	1293.34	test_vpc_vpn.py
test_01_vpc_site2site_vpn	`Error`	598.71	test_vpc_vpn.py

DaanHoogland · 2022-11-28T14:34:00Z

 cluster.node.IP=127.0.0.1
 cluster.servlet.port=9090


are these carriage returns intentionally left in?

Suggested change

cluster.node.IP=127.0.0.1

cluster.servlet.port=9090

cluster.node.IP=127.0.0.1

cluster.servlet.port=9090

I found an issue thread here pre-commit/pre-commit-hooks#196
which talks about the 50/50 rule and how to fix the line endings locally.

So that file had mostly Windows style carriage returns so did not get fixed at first.
Now I have just targeted Java and Python files for this initial first run clean up.

DaanHoogland · 2022-12-13T09:25:15Z

@blueorangutan package

blueorangutan · 2022-12-13T09:26:04Z

@DaanHoogland a 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.

codecov · 2022-12-20T10:25:59Z

Codecov Report

Merging #6273 (350ec92) into main (a3289f8) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               main    #6273   +/-   ##
=========================================
  Coverage     11.59%   11.59%           
  Complexity     7560     7560           
=========================================
  Files          2494     2494           
  Lines        247109   247109           
  Branches      38619    38619           
=========================================
  Hits          28646    28646           
  Misses       214719   214719           
  Partials       3744     3744

Impacted Files	Coverage Δ
.../cloud/baremetal/database/BaremetalRctDaoImpl.java	`0.00% <0.00%> (ø)`
...a/com/cloud/baremetal/database/BaremetalRctVO.java	`0.00% <0.00%> (ø)`
...java/com/cloud/baremetal/manager/BaremetalRct.java	`0.00% <0.00%> (ø)`
...ud/baremetal/manager/BaremetalVlanManagerImpl.java	`0.00% <0.00%> (ø)`
...baremetal/networkservice/BaremetalRctResponse.java	`0.00% <0.00%> (ø)`
...networkservice/BaremetalVirtualRouterCommands.java	`0.00% <0.00%> (ø)`
.../baremetal/networkservice/BaremetalVlanStruct.java	`0.00% <0.00%> (ø)`
.../networkservice/Force10BaremetalSwitchBackend.java	`0.00% <0.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

DaanHoogland · 2022-12-20T13:40:05Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2022-12-20T13:41:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

blueorangutan · 2022-12-21T02:02:04Z

Trillian test result (tid-5591)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43323 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t5591-vmware-67u3.zip
Smoke tests completed. 104 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_create_template_with_checksum_sha1	`Error`	5.14	test_templates.py
test_03_create_template_with_checksum_sha256	`Error`	5.13	test_templates.py
test_04_create_template_with_checksum_md5	`Error`	5.15	test_templates.py

DaanHoogland · 2022-12-21T08:51:16Z

Trillian test result (tid-5591) Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7 Total time taken: 43323 seconds Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t5591-vmware-67u3.zip Smoke tests completed. 104 look OK, 1 have errors, 0 did not run Only failed and skipped tests results shown below:
Test Result Time (s) Test File
test_02_create_template_with_checksum_sha1 Error 5.14 test_templates.py
test_03_create_template_with_checksum_sha256 Error 5.13 test_templates.py
test_04_create_template_with_checksum_md5 Error 5.15 test_templates.py

errors addressed in #7001

github-actions · 2023-01-03T14:00:24Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

DaanHoogland · 2023-01-09T14:37:01Z

@blueorangutan package

blueorangutan · 2023-01-09T14:38:03Z

@DaanHoogland a 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 · 2023-01-09T15:29:37Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5209

sonarqubecloud · 2023-01-09T15:34:06Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

DaanHoogland · 2023-01-09T16:54:27Z

@blueorangutan test matrix

blueorangutan · 2023-01-09T16:55:03Z

@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2023-01-10T05:55:07Z

Trillian test result (tid-5776)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45547 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t5776-xenserver-71.zip
Smoke tests completed. 106 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	1941.73	test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	1941.78	test_host_maintenance.py

blueorangutan · 2023-01-10T06:25:00Z

Trillian test result (tid-5777)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47219 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t5777-vmware-67u3.zip
Smoke tests completed. 105 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_attach_and_distribute_multiple_volumes	`Failure`	46.84	test_attach_multiple_volumes.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	664.10	test_kubernetes_clusters.py

blueorangutan · 2023-01-10T07:00:30Z

Trillian test result (tid-5778)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46590 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6273-t5778-kvm-centos7.zip
Smoke tests completed. 105 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	`Failure`	636.38	test_kubernetes_clusters.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	471.75	test_vpc_redundant.py

yadvr · 2023-01-10T08:24:34Z

@blueorangutan package

blueorangutan · 2023-01-10T08:25:03Z

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with

SystemVM template(s). I'll keep you posted as I make progress.

blueorangutan · 2023-01-10T09:13:11Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5223

davidjumani

Nice idea!

yadvr · 2023-01-13T15:21:39Z

Slightly concerned on changes outside of the commit/hook that I couldn't fully test/review, but LGTM.

boring-cyborg Bot added component:hyperv component:networking component:orchestration labels Apr 16, 2022

yadvr added this to the 4.18.0.0 milestone Apr 18, 2022

nvazquez modified the milestones: 4.18.0.0, 4.17.1.0 Apr 20, 2022

nvazquez force-pushed the main branch from c56220f to 0f4c3af Compare June 6, 2022 15:14

github-actions Bot added status:has-conflicts and removed status:has-conflicts labels Jul 1, 2022

shwstppr modified the milestones: 4.17.1.0, 4.18.0.0 Jul 29, 2022

github-actions Bot removed the status:has-conflicts label Aug 3, 2022

DaanHoogland approved these changes Aug 4, 2022

View reviewed changes

jbampton mentioned this pull request Nov 13, 2022

gha(python): fix W391 blank line at end of file #6897

Merged

12 tasks

DaanHoogland approved these changes Nov 28, 2022

View reviewed changes

boring-cyborg Bot added component:integration-test Python Warning... Python code Ahead! labels Dec 20, 2022

DaanHoogland requested a review from yadvr December 21, 2022 08:51

DaanHoogland added the needs-review label Dec 21, 2022

github-actions Bot added the status:has-conflicts label Jan 3, 2023

Merge branch 'main' into add-pre-commit-workflow

350ec92

boring-cyborg Bot added the component:integration label Jan 9, 2023

github-actions Bot removed the status:has-conflicts label Jan 9, 2023

davidjumani approved these changes Jan 11, 2023

View reviewed changes

DaanHoogland merged commit 00426ed into apache:main Jan 11, 2023

jbampton deleted the add-pre-commit-workflow branch January 11, 2023 16:51

Uh oh!

Conversation

jbampton commented Apr 16, 2022

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

github-actions Bot commented Jul 1, 2022

Uh oh!

github-actions Bot commented Jul 7, 2022

Uh oh!

DaanHoogland commented Jul 7, 2022

Uh oh!

shwstppr commented Jul 29, 2022

Uh oh!

jbampton commented Aug 4, 2022

Uh oh!

DaanHoogland commented Nov 10, 2022

Uh oh!

blueorangutan commented Nov 10, 2022

Uh oh!

blueorangutan commented Nov 10, 2022

Uh oh!

DaanHoogland commented Nov 13, 2022

Uh oh!

blueorangutan commented Nov 13, 2022

Uh oh!

blueorangutan commented Nov 13, 2022

Uh oh!

blueorangutan commented Nov 13, 2022

Uh oh!

DaanHoogland Nov 28, 2022

Choose a reason for hiding this comment

Uh oh!

jbampton Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Dec 13, 2022

Uh oh!

blueorangutan commented Dec 13, 2022

Uh oh!

codecov Bot commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Dec 20, 2022

Uh oh!

blueorangutan commented Dec 20, 2022

Uh oh!

blueorangutan commented Dec 21, 2022

Uh oh!

DaanHoogland commented Dec 21, 2022

Uh oh!

github-actions Bot commented Jan 3, 2023

Uh oh!

DaanHoogland commented Jan 9, 2023

Uh oh!

blueorangutan commented Jan 9, 2023

Uh oh!

blueorangutan commented Jan 9, 2023

Uh oh!

sonarqubecloud Bot commented Jan 9, 2023

Uh oh!

DaanHoogland commented Jan 9, 2023

Uh oh!

blueorangutan commented Jan 9, 2023

Uh oh!

blueorangutan commented Jan 10, 2023

Uh oh!

blueorangutan commented Jan 10, 2023

Uh oh!

blueorangutan commented Jan 10, 2023

Uh oh!

yadvr commented Jan 10, 2023

Uh oh!

blueorangutan commented Jan 10, 2023

codecov Bot commented Dec 20, 2022 •

edited

Loading