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

Docker hook was removed by bot #13

Open
flying-sheep opened this issue Feb 6, 2024 · 6 comments
Open

Docker hook was removed by bot #13

flying-sheep opened this issue Feb 6, 2024 · 6 comments
Assignees

Comments

@flying-sheep
Copy link
Contributor

flying-sheep commented Feb 6, 2024

What happened?

in 01c5701, the docker hook was removed.

Relevant log output

No response

Is it possible to reproduce the bug with a small code snippet?

$ pre-commit autoupdate
[https://github.com/ComPWA/mirrors-taplo] Cannot update because the update target is missing these hooks: taplo-docker

Additional steps to reproduce the bug

No response

Which Python version were you using?

None

Python dependencies

No response

@redeboer
Copy link
Member

redeboer commented Feb 6, 2024

Hmm curious indeed... Also how did the rev end up there in the .pre-commit-config.yaml? 😬

At any rate, there are some upstream problems as well with v0.9.0 (tamasfe/taplo#542) that seem to be similar to problems with the Docker build for this mirror repo.

@redeboer
Copy link
Member

redeboer commented Feb 6, 2024

Tbh I haven't been happy with this mirror since the beginning and would have preferred to see a taplo Python package with prebuilt wheels, which would make it trivial to host a (much faster) pre-commit hook. Have played around a bit with building wheels for Taplo with Maturin and I'll make a PR to Taplo once I have time to get back to that.

If you're interested, you can already try a test pip package:
https://pypi.org/project/taplo-test

And here's the corresponding (temporary!) pre-commit hook:

  - repo: https://github.com/redeboer/taplo-pre-commit
    rev: v0.9.1rc1
    hooks:
      - id: taplo

Interested to hear if that hook works for you, but don't check it in to your CI ;)

@flying-sheep
Copy link
Contributor Author

flying-sheep commented Feb 7, 2024

That’s exactly the solution I thought about, awesome job of getting it working!

Taplo upstream is currently in a bit of chaos since new maintainers are taking over and haven’t figured out all the processes, but eventually we could upstream your workflows so all the package building is centralized.

/edit: your changes are pretty neat! I’d not add the kitchen sink to .gitignore, otherwise that looks like a great solution.

My .gitignores usually look like:

__pycache__/
/.*cache/
/dist/

# and if others work on it too:
/.vscode/
/.idea/
/.python-version
/.venv/

@maresb
Copy link

maresb commented Feb 7, 2024

Hey, this looks awesome!!!

A few days ago I was asking about an official pre-commit hook here: tamasfe/taplo#535

@maresb
Copy link

maresb commented Feb 7, 2024

Just had a closer look at the commits. That looks like it was way easier that I could have imagined, up until the OpenSSL stuff.

I was wondering why you'd need OpenSSL, and found the answer here.

Taplo depends on OpenSSL in order to fetch schemas via HTTPS, you will most likely need the openssl development files to be installed (openssl-dev or openssl-devel on most Linux-based systems).

But then it looks like the dependency was removed in 0.7.0. I suppose it was readded later? I'm not familiar with the taplo codebase or rust programming, so my explorations and questions may be quite naive.

@redeboer
Copy link
Member

redeboer commented Feb 7, 2024

Thanks a lot for the feedback, great to hear it works!

Just had a closer look at the commits. That looks like it was way easier that I could have imagined, up until the OpenSSL stuff.

Well spotted indeed 😅 That was the painful part and actually what held me back from making a PR yet. Two things:

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

No branches or pull requests

3 participants