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

feat(bux-246): Added linters #500

Merged
merged 34 commits into from
Apr 12, 2024
Merged

feat(bux-246): Added linters #500

merged 34 commits into from
Apr 12, 2024

Conversation

Nazarii-4chain
Copy link
Contributor

@Nazarii-4chain Nazarii-4chain commented Mar 12, 2024

Pull Request Checklist

  • πŸ“– I created my PR using provided : CODE_STANDARDS
  • πŸ“– I have read the short Code of Conduct: CODE_OF_CONDUCT
  • 🏠 I tested my changes locally.
  • βœ… I have provided tests for my changes.
  • πŸ“ I have used conventional commits.
  • πŸ“— I have updated any related documentation.
  • πŸ’Ύ PR was issued based on the Github or Jira issue.

@Nazarii-4chain Nazarii-4chain requested a review from a team as a code owner March 12, 2024 12:21
Copy link

github-actions bot commented Mar 12, 2024

Manual Tests

πŸ’š Manual testing by @arkadiuszos4chain resulted in success.

@codecov-commenter
Copy link

codecov-commenter commented Mar 12, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 53.08%. Comparing base (8b7d055) to head (5cc309f).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #500   +/-   ##
=======================================
  Coverage   53.08%   53.08%           
=======================================
  Files         193      193           
  Lines       10895    10895           
=======================================
  Hits         5784     5784           
  Misses       4561     4561           
  Partials      550      550           
Flag Coverage Ξ”
unittests 53.08% <ΓΈ> (ΓΈ)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Ξ”
config/config.go 100.00% <ΓΈ> (ΓΈ)
dictionary/dictionary.go 95.12% <ΓΈ> (ΓΈ)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data
Powered by Codecov. Last update 8b7d055...5cc309f. Read the comment docs.

@Nazarii-4chain Nazarii-4chain self-assigned this Mar 12, 2024
Copy link
Contributor

@chris-4chain chris-4chain left a comment

Choose a reason for hiding this comment

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

I guess we should schedule a team meeting to discuss the whole topic.
Some brief summary (maybe as description of this PR) would be nice.

@Nazarii-4chain
Copy link
Contributor Author

If everything is good. Will remove golangci.yml

.golangci.yml Outdated Show resolved Hide resolved
.github/workflows/error-linter.yml Outdated Show resolved Hide resolved
.github/workflows/code-style-linter.yml Outdated Show resolved Hide resolved
.github/workflows/error-linter.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@arkadiuszos4chain arkadiuszos4chain left a comment

Choose a reason for hiding this comment

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

What changed for yamllint to start throwing warnings and treating them as errors?

@Nazarii-4chain
Copy link
Contributor Author

yamllint

It seems that yamllint have not checked these files

@arkadiuszos4chain
Copy link
Contributor

It seems that yamllint have not checked these files

Then we should configure yamlint to display warnings and treat them as such, not as errors. I don't like idea of adding '---' at the beginning of every YAML when we're not using directives.
Also truthy should be configured to allow 'on' value since it's a know name of triggers section

@Nazarii-4chain
Copy link
Contributor Author

It seems that yamllint have not checked these files

Then we should configure yamlint to display warnings and treat them as such, not as errors. I don't like idea of adding '---' at the beginning of every YAML when we're not using directives. Also truthy should be configured to allow 'on' value since it's a know name of triggers section

Changed

@arkadiuszos4chain arkadiuszos4chain added the tested PR was tested by a team member label Apr 12, 2024
@arkadiuszos4chain arkadiuszos4chain added tested PR was tested by a team member and removed tested PR was tested by a team member labels Apr 12, 2024
@Nazarii-4chain Nazarii-4chain merged commit 271d217 into main Apr 12, 2024
8 of 9 checks passed
@Nazarii-4chain Nazarii-4chain deleted the BUX-246/Linters branch April 12, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR was tested by a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants