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

Auto-format css/js/html and add eslint #294

Merged
merged 7 commits into from
Sep 27, 2024
Merged

Conversation

balazs-endresz
Copy link
Contributor

  • Are tests passing? (From the root-level of the repository please run pip install tox && tox)
  • I have added or updated a test to cover the changes proposed in this Pull Request
  • I have updated the documentation to cover the changes proposed in this Pull Request

Tests passed locally with tox and I these shouldn't require any test or docs changes. Also tested manually with the testproject.


This PR adds auto formatting with pre-commit for js/css/html, and some minor linting tweaks:

  • Fixed flake8 url, which has been changed a while ago from gitlab to github
  • Reformatted rosetta.css and rosetta.js with prettier
  • Added eslint config (using v8 because couldn't get the newer v9 config working)
  • Fixed a few eslint warnings in rosetta.js
  • Reformatted django templates with djhtml
  • Added trimmed to blocktrans in test.html since the indentation was changed, but I guess this would be done in practice too to ignore the newlines.
  • prettier works for yaml files too but not added that here

I tried to stick to the defaults mostly, but I'm happy to redo this with different settings if needed.

@balazs-endresz
Copy link
Contributor Author

Rebased and added some minor ruff fixes that came up after pre-commit run --all-files. (I guess those went unnoticed after adding ruff because pre-commit checks only modified files by default.)

@mbi
Copy link
Owner

mbi commented Sep 27, 2024

Hello @balazs-endresz

Trying to run pre-commit yields this error. Could you please provide a minimal config file, ideally as a hidden dotfile in the root of the project?

Oops! Something went wrong! :(

ESLint: 8.56.0

ESLint couldn't find a configuration file. To set up a configuration file for this project, please run:

npm init @eslint/config

ESLint looked for configuration files in /home/marco/Code/django-rosetta/rosetta/static/admin/rosetta/js and its ancestors. If it found none, it then looked in your home directory.

If you think you already have a configuration file or if you need more help, please stop by the ESLint Discord server: https://eslint.org/chat

@balazs-endresz
Copy link
Contributor Author

balazs-endresz commented Sep 27, 2024

Turns out it worked for me because I had a default eslint config file in my home directory. I tried again with eslint version 9 but still couldn't get that working with prettier pre-commit, but at least the older v8 config file should work now with a new .eslintrc.js file added to the first commit.

@mbi mbi merged commit 88bc11d into mbi:develop Sep 27, 2024
@mbi
Copy link
Owner

mbi commented Sep 27, 2024

Thank you, merged!

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