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

Added linters to the project #473

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

BassCoder2808
Copy link
Contributor

@BassCoder2808 BassCoder2808 commented Jun 3, 2023

  1. Python linter: pylint (pip install pylint)
  2. JavaScript linter: eslint (npm install eslint)
  3. HTML linter: htmlhint
  4. CSS linter: stylelint (npm install stylelint)
    • Create configuration files for each linter:
  5. Python: create a .pylintrc file in the root directory of the project with desired configuration
    options
  6. JavaScript: create an .eslintrc file in the root directory of the project with desired configuration
    options.
  7. HTML: create a .tidyrc file in the root directory of the project with desired configuration options
  8. CSS: create a stylelint.config.js file in the root directory of the project with desired configuration
    options.
    • Integrate the linters into the project workflow:
  9. For Python, add a pylint command to the project's Makefile or scripts directory.
  10. For JavaScript, add an eslint command to the project's Makefile or scripts directory and include it
    in the pre-commit hook.
  11. For HTML, add a tidy command to the project's Makefile or scripts directory and include it in the
    pre-commit hook.
  12. For CSS, add a stylelint command to the project's Makefile or scripts directory and include it in
    the pre-commit hook.

@@ -0,0 +1,28 @@
[MESSAGES CONTROL]
Copy link
Collaborator

@rtgdk rtgdk Aug 24, 2023

Choose a reason for hiding this comment

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

Where's the change when these linters are applied to all the files in the project? I think I will delay merging this just to have less conflicts with other open PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had just made the files and ran them only on few files and not all. Should I run them on all the files?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, run all the test. We need to confirm everything works fine after your changes land.

Copy link
Collaborator

@rtgdk rtgdk Oct 3, 2023

Choose a reason for hiding this comment

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

@BassCoder2808 Did you run the tests for this after linting? Also make sure the app works fine and there are no code leakages.

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.

2 participants