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

Ail 162/feature/setup precommit basic linting #95

Merged

Conversation

rodrigo-pessoa-lrn
Copy link
Contributor

@rodrigo-pessoa-lrn rodrigo-pessoa-lrn commented Nov 1, 2024

Problem Statement

Add "easy win" pre-commit rules to help to catch simple errors and reduce whitespace differences in PRs as well.

PR Actions - What was done

  • Added "easy win" pre-commit rules:
    • end-of-file-fixer
    • trailing-whitespace
    • check-yaml
    • check-json
    • pretty-format-json
    • check-merge-conflict
    • check-symlinks
    • detect-private-key

How was this change tested

  • Ran test suite.
  • Ran coverage checker.
  • Ran learnosity-sdk-assessment-quickstart with local build.
  • Ensured that the Github CI action was present when PR was opened and that it was blocking if failing.

Notes:

N/A

@rodrigo-pessoa-lrn rodrigo-pessoa-lrn force-pushed the AIL-162/feature/setup_precommit_basic_linting branch 7 times, most recently from 3255434 to 1377e1d Compare November 1, 2024 14:06
@rodrigo-pessoa-lrn
Copy link
Contributor Author

rodrigo-pessoa-lrn commented Nov 1, 2024

Note for reviewers
These are the key files that were modified / added:

  • .github/workflows/ci.yml
  • .pre-commit-config.yaml
  • ChangeLog.md
  • setup.py

All other files were modified according to the pre-commit hook rules (basically removal of trailing white spaces, additional of blank line at the bottom of every file and fixing typos in comments). These changes do not affect any logic or behaviour.

@rodrigo-pessoa-lrn rodrigo-pessoa-lrn force-pushed the AIL-162/feature/setup_precommit_basic_linting branch 2 times, most recently from 3ad6bd6 to 35324c1 Compare November 1, 2024 16:57
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn marked this pull request as ready for review November 1, 2024 17:07
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn force-pushed the AIL-162/feature/setup_precommit_basic_linting branch from 35324c1 to 2c03fbd Compare November 4, 2024 11:18
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn marked this pull request as draft November 4, 2024 13:55
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn removed the request for review from sebablanco November 4, 2024 13:55
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn force-pushed the AIL-162/feature/setup_precommit_basic_linting branch from 2c03fbd to e45214c Compare November 4, 2024 17:10
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn force-pushed the AIL-162/feature/setup_precommit_basic_linting branch from b20de66 to facfdb2 Compare November 4, 2024 17:24
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn marked this pull request as ready for review November 4, 2024 17:27
Copy link
Contributor

Choose a reason for hiding this comment

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

👏 praise: Looking good! Nice to no longer have whitespace issues 🚀

Some very minor comments and then ready to approve

setup.py Outdated Show resolved Hide resolved
ChangeLog.md Outdated Show resolved Hide resolved
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn merged commit 6477330 into master Nov 5, 2024
7 checks passed
@rodrigo-pessoa-lrn rodrigo-pessoa-lrn deleted the AIL-162/feature/setup_precommit_basic_linting branch November 5, 2024 11:30
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.

3 participants