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

Add pre-commit hooks for code checks #6407

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

mahesh-naxa
Copy link
Contributor

@mahesh-naxa mahesh-naxa commented May 3, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Describe this PR

We had no mechanism to perform checks for the code that has been pushed as mentioned #5714 . Also since the integration of pre-commit.ci we have been facing checks failing due to lack of pre-commit-config.yaml within the project.

Need of Further Investigation (⚠️ )

  • For https://github.com/pre-commit/mirrors-prettier checks, autofix didn't fix all the issues so fronend team should have to look into depth. Currently is commented out.
  • https://github.com/asottile/dead similarly had also flagged many portion of the code that were unused, But this should be confirmed by dev team & then we can have this enabled as well.
  • for pretty-format-json hook, that would format all json files, this caused tests to fail, i believe this is due to some test cases that wouldn't parse successfully. Needs more investigation from backend team, as just formatting the *.json the tests shouldn't have failed.

Screenshots

Screenshot 2024-05-03 at 9 42 15β€―AM Screenshot 2024-05-03 at 10 10 41β€―AM

Copy link

sonarcloud bot commented May 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mahesh-naxa
Copy link
Contributor Author

@eternaltyro the build and push seems to fail, if reran. It did pass the first time i created this PR, but failed once i had more commits pushed to this branch. Just letting you know.

@mahesh-naxa
Copy link
Contributor Author

@dakotabenjamin i have 3 of those commented since they messed up our code and autofix didn't work. Need more input from dev end on those. May be we should create new issues about that.

Copy link
Member

@dakotabenjamin dakotabenjamin left a comment

Choose a reason for hiding this comment

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

99% of the changes are extraneous newlines, spaces, and tabs. the other 1% look good and agree on the commented out files needing input from the other devs. great start! LGTM

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

Successfully merging this pull request may close these issues.

None yet

2 participants