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

Setup pre-commit on this project #267

Open
humitos opened this issue Feb 20, 2024 · 4 comments
Open

Setup pre-commit on this project #267

humitos opened this issue Feb 20, 2024 · 4 comments
Assignees
Labels
Improvement Minor improvement to code

Comments

@humitos
Copy link
Member

humitos commented Feb 20, 2024

We should configure pre-commit on this project to call all the validations that are happening on CircleCI and avoid unexpected failures after pushing the code to GitHub.

@humitos humitos added the Improvement Minor improvement to code label Feb 20, 2024
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Feb 20, 2024
@humitos humitos self-assigned this Feb 28, 2024
@humitos
Copy link
Member Author

humitos commented Mar 5, 2024

I jumped into this today and I found it's going to be pretty complex to setup this, and maybe a little tedious. Mainly because on CircleCI we are performing a check over the HTML output generated by Pelican.

https://github.com/readthedocs/website/blob/main/.circleci/config.yml#L34

So, we will need to build the site on pre-commit to be able to replicate that locally, which seems a big overhead added. We would need to install dependencies via Poetry as well, and more complex things.

Unfortunately, I think there is no easy answer here and it may makes sense to de-prioritize this task for now, since it won't have too much impact anyways.

The other checks are more related to the custom Pelican theme we have here, which we are not touching frequently --so, it's probably fine to skip them for now.

@humitos humitos moved this from Planned to In progress in 📍Roadmap Mar 5, 2024
@agjohnson
Copy link
Contributor

Are there any specific pre-commit checks we wanted? I would agree this doesn't seem super valuable at the moment with all the current linting/checks providing a good amount of value already.

@humitos
Copy link
Member Author

humitos commented Mar 7, 2024

Are there any specific pre-commit checks we wanted?

The one that I want is npx html-validate --formatter codeframe output/ which is the hardest to setup (requires building the site with Pelican + poetry), and the one why I stopped this work 😄 . Most of the other pre-commit checks we could add are not super relevant.

@agjohnson
Copy link
Contributor

Yeah that makes sense.

I think I might have thought you were implying somewhere that we have existing pre-commit checks in common/etc that would be applicable in this repo. Adding pre-commit would be nice, but this is mostly used when editing HTML too, so less frequent than blog posts/markdown at least. I'd agree it would be nice but not a strong priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code
Projects
Status: In progress
Development

No branches or pull requests

2 participants