KEMBAR78
add lint id enum (#426) by demfabris · Pull Request #427 · dotenv-linter/dotenv-linter · GitHub
Skip to content

add lint id enum (#426) #427

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Jun 30, 2021
Merged

add lint id enum (#426) #427

merged 16 commits into from
Jun 30, 2021

Conversation

@demfabris
Copy link
Contributor

Began refactoring src/checks.rs

  • add child module check_variants
  • add Lint variants
  • refactor test setup to support the variants

Note: this is just a checkpoint to see if i'm on the right track. Code does not compile yet!

@mgrachev
Copy link
Member

@fabricio7p 👋 Thank you for your contribution 👍

Let us know when you've finished so we can review your PR 😉

@demfabris
Copy link
Contributor Author

demfabris commented Jun 1, 2021 •
edited
Loading

Checkpoint.

  • add CheckVariants struct that hold the variants vec. Vec<Lint>
  • refactor comment.rs to support output the new CheckVariants struct
  • begin refactoring each /src/fixes (only first 3 done)

edit: I should probably rename CheckVariantsto Checks... just wondering

@demfabris demfabris force-pushed the refactor/lint-identity branch from b12eca0 to 173e1f3 Compare June 6, 2021 00:24
@demfabris
Copy link
Contributor Author

Refactor done.
Please review @mgrachev

  • Refactored every occurence of string comparisons to support the new LintKind enum.
  • Refactored all tests to support enum variants
  • Moved Lint and LintKind to new lints.rs file
  • Add testing to Lint and LintKind

@mgrachev mgrachev requested review from DDtKey and mgrachev June 7, 2021 08:14
@mgrachev
Copy link
Member

mgrachev commented Jun 8, 2021 •
edited
Loading

Thank you 👍

I'm sorry, I'm quite busy now, so I'll try to review your PR by the end of the week 🙂

@mgrachev mgrachev linked an issue Jun 8, 2021 that may be closed by this pull request
Use enum instead of string for lint's identity #426
Closed
mgrachev
mgrachev reviewed Jun 22, 2021
View reviewed changes
Copy link
Member

@mgrachev mgrachev left a comment

Choose a reason for hiding this comment

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

@fabricio7p Thank you for your contribution 🚀

I'm sorry for the late reply.
I have left some comments.
Please take a look at them 👀

@demfabris demfabris force-pushed the refactor/lint-identity branch from 173e1f3 to 353307d Compare June 26, 2021 14:15
@demfabris
Copy link
Contributor Author

Alright, everything refactored.
Rebased from master as well.

mgrachev
mgrachev reviewed Jun 29, 2021
View reviewed changes
mgrachev
mgrachev reviewed Jun 29, 2021
View reviewed changes
@demfabris
Copy link
Contributor Author

All good.
Tests passing 👍

mgrachev
mgrachev previously approved these changes Jun 30, 2021
View reviewed changes
@codecov-commenter
Copy link

Codecov Report

Merging #427 (c237294) into master (7237b17) will increase coverage by 0.00%.
The diff coverage is 98.95%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #427    +/-   ##
========================================
  Coverage   98.94%   98.95%            
========================================
  Files          39       40     +1     
  Lines        2664     2765   +101     
========================================
+ Hits         2636     2736   +100     
- Misses         28       29     +1
Impacted Files Coverage Δ
src/lib.rs 97.84% <75.00%> (-0.69%) ⬇️
src/fixes.rs 92.61% <93.33%> (+0.04%) ⬆️
src/checks.rs 100.00% <100.00%> (ø)
src/checks/duplicated_key.rs 100.00% <100.00%> (ø)
src/checks/ending_blank_line.rs 100.00% <100.00%> (ø)
src/checks/extra_blank_line.rs 100.00% <100.00%> (ø)
src/checks/incorrect_delimiter.rs 100.00% <100.00%> (ø)
src/checks/key_without_value.rs 100.00% <100.00%> (ø)
src/checks/leading_character.rs 100.00% <100.00%> (ø)
src/checks/lowercase_key.rs 100.00% <100.00%> (ø)
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7237b17...c237294. Read the comment docs.

@mgrachev
Copy link
Member

mgrachev commented Jun 30, 2021 •
edited
Loading

@fabricio7p Thanks a lot 🔥

DDtKey
DDtKey previously approved these changes Jun 30, 2021
View reviewed changes
Copy link
Member

@DDtKey DDtKey left a comment

Choose a reason for hiding this comment

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

Thank you for very useful PR 🙂

@mgrachev mgrachev dismissed stale reviews from DDtKey and themself via 414fc6f June 30, 2021 09:27
@mgrachev mgrachev merged commit 313f329 into dotenv-linter:master Jun 30, 2021
@mgrachev
Copy link
Member

@fabricio7p Thank you for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgrachev mgrachev mgrachev left review comments

@DDtKey DDtKey DDtKey left review comments

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use enum instead of string for lint's identity

4 participants

@demfabris @mgrachev @codecov-commenter @DDtKey