Skip to content

Refactor Qcode CI to Combine Jobs#12

Merged
bvw merged 7 commits into
masterfrom
refactor-qcode-ci
Feb 23, 2023
Merged

Refactor Qcode CI to Combine Jobs#12
bvw merged 7 commits into
masterfrom
refactor-qcode-ci

Conversation

@nicky-johnstone
Copy link
Copy Markdown
Member

@nicky-johnstone nicky-johnstone commented Feb 23, 2023

Successful run:
https://github.com/qcode-software/ci-development/actions/runs/4252744259/jobs/7396712578

Run with some steps failing due to linting not passing:
https://github.com/qcode-software/ci-development/actions/runs/4252725650/jobs/7396670208

Motivation for Change

If the qcode-ci workflow is run in a private repository then GitHub bills for time taken to run each job with a minimum billing of 1 minute if I recall correctly.

The current setup will run 5 different jobs (one for each linting check) and while the Tcl scripts take trivial time to run the set up for each job takes ~30 seconds meaning a typical bill per run in a private repository is 5 minutes.

This new setup only runs one job and all of the linting checks get to reuse the setup therefore a bill for a typical run in a private repository should be reduced to just 1 minute.

Testing

tclsh tcl/linter.test 
linter.test:	Total	6	Passed	6	Skipped	0	Failed	0

id: check-line-length
if: >
always()
&& steps.changed-files.outcome == 'success'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This has been added to every linting step so that if one linting step fails then it will not prevent any other linting steps from running.

always() makes the step always run regardless of whether the job has been cancelled or if any previous steps have failed.

&& steps.changed-files.outcome == 'success' checks that the changed files action has succeeded. This is important because we only really want each linting step to run if the shared setup steps (i.e. dependency installation, repo checkouts, and ) have been successful. If the last step in the shared setup successfully runs then it means that the steps prior to it have also successfully run.

@nicky-johnstone
Copy link
Copy Markdown
Member Author

Examples of Feedback

Check Proc Body Lengths

Checking for procs that have bodies that are more than 10 lines long.
The proc "linter_report_lines_over_length" in file tcl/linter.tcl has a body that is 30 long.
The proc "linter_report_procs_without_filename_prefix" in file tcl/linter.tcl has a body that is 27 long.
The proc "linter_proc_names_parse" in file tcl/linter.tcl has a body that is 11 long.
The proc "linter_proc_lengths" in file tcl/linter.tcl has a body that is 23 long.
The proc "test_coverage_file_line_matches_get" in file tcl/test_coverage.tcl has a body that is 20 long.

Check File Lengths

Checking for files that are more than 10 lines long.
The file tcl/linter.tcl has 187 lines.
The file tcl/test_coverage.tcl has 278 lines.

Check Proc Name Prefixes

Checking proc name prefixes in files that have changed.
The proc name "test_1" in file coverage_test.tcl is not prefixed with "coverage_test".
The proc name "test_2" in file coverage_test.tcl is not prefixed with "coverage_test".
The proc name "test_3" in file coverage_test.tcl is not prefixed with "coverage_test".

Check Each Proc Has A Unit Test

Checking for procs that do not have at least one unit test.
The proc "cast_integer" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_int" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_decimal" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_date" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_timestamp" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_timestamptz" in file tcl/cast.tcl doesn't have a unit test.
The proc "cast_epoch" in file tcl/cast.tcl doesn't have a unit test.

Check Line Lengths

Checking line length is under 75 chars in files that have changed.
Line 16 in file /home/nick/ci-development/tcl/linter.tcl is 76 chars long - set partial_line...
Line 18 in file /home/nick/ci-development/tcl/linter.tcl is 84 chars long - set details "Lin...
Line 29 in file /home/nick/ci-development/tcl/linter.tcl is 84 chars long - error $message [...
Line 52 in file /home/nick/ci-development/tcl/linter.tcl is 86 chars long - lappend report "...
Line 59 in file /home/nick/ci-development/tcl/linter.tcl is 84 chars long - error $message [...
Line 108 in file /home/nick/ci-development/tcl/linter.tcl is 78 chars long - set command [str...
Line 145 in file /home/nick/ci-development/tcl/linter.tcl is 80 chars long - #| Returns proc ...

@bvw bvw merged commit be72564 into master Feb 23, 2023
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.

2 participants