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 #140

Merged
merged 2 commits into from
May 26, 2021
Merged

Add pre-commit #140

merged 2 commits into from
May 26, 2021

Conversation

shubhank-saxena
Copy link
Contributor

This fixes #131
@mgogoulos please let me know if there are any further changes required.
Thank you!

@swiftugandan
Copy link
Contributor

You can also extend this further and add a github action. See: https://levelup.gitconnected.com/raise-the-bar-of-code-quality-in-python-projects-7c49743f004f

@shubhank-saxena
Copy link
Contributor Author

shubhank-saxena commented Apr 6, 2021

You can also extend this further and add a github action. See: https://levelup.gitconnected.com/raise-the-bar-of-code-quality-in-python-projects-7c49743f004f

This was the next thing on my mind. Will do this. Thank you! @swiftugandan

@shubhank-saxena
Copy link
Contributor Author

@swiftugandan done! Please add a secret in the repo settings named CI_TOKEN. This should be a Personal Access Token allowing the Read/Write commit.

@swiftugandan
Copy link
Contributor

  • I was expecting one file ie. .github/workflows/lint_test.yml
  • What is this file used for? .github/lint_test.yml
  • According to https://github.com/pre-commit/action, we should use ${{ secrets.GITHUB_TOKEN }} and its automatically provisioned, we dont have to do anything

@shubhank-saxena
Copy link
Contributor Author

* I was expecting one file ie. `.github/workflows/lint_test.yml`

* What is this file used for? `.github/lint_test.yml`

* According to https://github.com/pre-commit/action, we should use `${{ secrets.GITHUB_TOKEN }}` and its automatically provisioned, we dont have to do anything
  • Added another file by mistake.
  • I did try this once on one of my setups. Was causing some trouble. Will try it here once again.

@shubhank-saxena
Copy link
Contributor Author

@swiftugandan fixed!

@mgogoulos
Copy link
Contributor

Thanks for this! Looks nice but want to perform some more testing before it gets merged

@mgogoulos
Copy link
Contributor

Ok most probably lots of changes need to be done after merging this, but seems like a good start. Merging

@mgogoulos mgogoulos merged commit 8f228d6 into mediacms-io:main May 26, 2021
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.

[Tool/Feature] Add pre-commit to ensure the code quality
3 participants