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

refactor: move lint logic from pacakger to command #2697

Closed
wants to merge 1 commit into from

Conversation

phillebaba
Copy link
Member

Description

This moves the CLI specific lint logic out of the packager and into the command instead. This means that you do not need a packager instance to do the linting. Additionally this removes the use of os.Chdir for the linter.

Doing this reduces the complexity as the linter logic does not need to know about all other packager related logic which currently exists.

Related Issue

Relates to #2694

Checklist before merging

@phillebaba phillebaba requested review from dgershman and a team as code owners July 3, 2024 14:12
Copy link

netlify bot commented Jul 3, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 6519be7
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6686acc03ae2670008d6c8fc

@phillebaba phillebaba force-pushed the refactor/lint branch 2 times, most recently from 471be96 to 244b073 Compare July 3, 2024 14:22
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 4.08163% with 47 lines in your changes missing coverage. Please review.

Project coverage is 19.01%. Comparing base (014d1d2) to head (6519be7).

Files Patch % Lines
src/cmd/dev.go 0.00% 40 Missing ⚠️
src/pkg/packager/lint/lint.go 22.22% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2697      +/-   ##
==========================================
+ Coverage   19.00%   19.01%   +0.01%     
==========================================
  Files         170      170              
  Lines       12197    12190       -7     
==========================================
  Hits         2318     2318              
+ Misses       9593     9586       -7     
  Partials      286      286              

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

@AustinAbro321
Copy link
Contributor

@phillebaba This gets covered by #2585, albeit in a different way. The code moved into cmd here is instead moved to the lint package. I believe this should be closed, however if you want to get this merged in first or prefer having the logic in cmd rather than lint lmk.

@phillebaba
Copy link
Member Author

You are right, closing in favor of your current work.

@phillebaba phillebaba closed this Jul 11, 2024
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