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

qa: Enforce JSON linter for GCT configs #1526

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

thrasher-
Copy link
Collaborator

PR Description

Adds a new GHA and Makefile utility command to enforce JSON format standards for our configs using jq. Also sorts the exchanges by name for now rather than the whole file for starters.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested

  • go test ./... -race
  • golangci-lint run
  • GHA
  • make lint_configs and manual breaking to make sure the diff wigs out

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Github Actions with my changes
  • Any dependent changes have been merged and published in downstream modules

@thrasher- thrasher- added the review me This pull request is ready for review label Apr 26, 2024
@thrasher- thrasher- requested a review from a team April 26, 2024 05:22
@thrasher- thrasher- self-assigned this Apr 26, 2024
Copy link

codecov bot commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.03%. Comparing base (44d50a3) to head (cfecfd7).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1526      +/-   ##
==========================================
+ Coverage   36.02%   36.03%   +0.01%     
==========================================
  Files         411      411              
  Lines      176891   176914      +23     
==========================================
+ Hits        63726    63757      +31     
+ Misses     105384   105369      -15     
- Partials     7781     7788       +7     

see 25 files with indirect coverage changes

Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

Tickity Tack.


- name: Check configs JSON format
run: |
files=("config_example.json" "testdata/configtest.json")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: might be able to use files=$(git ls-files '*.json') so we can conform everything and for future files and filter by config key word. But that seems like a pain. So only a suggestion.

here's the output:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, it was made to be extensible but this PR only targets our mainly touched configs for now

Copy link
Collaborator

@gloriousCode gloriousCode left a comment

Choose a reason for hiding this comment

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

tACK!

make lint_configs
Checking if jq is installed...
Processing config_example.json...
jq: parse error: Invalid numeric literal at line 16, column 0
jq processing failed on *.json
make: *** [lint_configs] Error 1

make lint_configs
Checking if jq is installed...
Processing config_example.json...
Processing testdata/configtest.json...

Confirmed that it works 🌞 Emoji's inside yml files is approved too

Copy link
Collaborator

@gloriousCode gloriousCode left a comment

Choose a reason for hiding this comment

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

retACK! Increase in actual human friendliness detected

make lint_configs
-n Checking if jq is installed... 
OK
-n Processing config_example.json... 
OK
-n Processing testdata/configtest.json... 
OK

make lint_configs
-n Checking if jq is installed... 
OK
-n Processing config_example.json... 
OK
-n Processing testdata/configtest.json... 
jq: parse error: Invalid numeric literal at line 462, column 0
FAILED
make: *** [lint_configs] Error 1

Copy link
Collaborator

@shazbert shazbert left a comment

Choose a reason for hiding this comment

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

ch tack

Copy link
Collaborator

@gloriousCode gloriousCode left a comment

Choose a reason for hiding this comment

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

make lint_configs
Checking if jq is installed... OK
Processing config_example.json... OK
Processing testdata/configtest.json... OK
make lint_configs
Checking if jq is installed... OK
Processing config_example.json... OK
Processing testdata/configtest.json... jq: parse error: Invalid numeric literal at line 462, column 0
FAILED
make: *** [lint_configs] Error 1

Now with less -n

@thrasher- thrasher- merged commit 1e95ae9 into thrasher-corp:master Apr 30, 2024
10 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review me This pull request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants