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

feat: support docker language #67

Merged
merged 24 commits into from
Nov 20, 2024
Merged

feat: support docker language #67

merged 24 commits into from
Nov 20, 2024

Conversation

bxb100
Copy link
Contributor

@bxb100 bxb100 commented Nov 15, 2024

close: #49

TODO

  • add test suits
  • test inner container
    • disable MacOS test, because it's without docker1
    • disable Windows test, because it's without linux kernel2
  • test windows

Footnotes

  1. https://github.com/actions/runner-images/issues/17

  2. https://github.com/orgs/community/discussions/21317

Cargo.toml Outdated Show resolved Hide resolved
@j178
Copy link
Owner

j178 commented Nov 17, 2024

Thanks for your fantastic work! I'll review it soon!

@bxb100 bxb100 marked this pull request as ready for review November 17, 2024 11:58
@bxb100 bxb100 marked this pull request as draft November 17, 2024 15:24
@bxb100
Copy link
Contributor Author

bxb100 commented Nov 17, 2024

@j178 I want to rewrite all commands to this crate https://github.com/fussybeaver/bollard , so maybe late review /= =/

@j178
Copy link
Owner

j178 commented Nov 17, 2024

@bxb100 I would prefer using the Docker CLI because it is lightweight and doesn't require many dependencies for basic Docker operations, unlike bollard, which seems a bit heavy to me. What do you think?

@bxb100
Copy link
Contributor Author

bxb100 commented Nov 17, 2024

@j178 Ok

@bxb100 bxb100 marked this pull request as ready for review November 17, 2024 16:33
@j178
Copy link
Owner

j178 commented Nov 20, 2024

Thanks for your fantastic work here! I'm planning to make some adjustments to the coding style, hope you don't mind. Also, I'm considering simplifying the Docker test by removing the inner Docker test. It seems unnecessary to add a complex test job just for that.

@j178
Copy link
Owner

j178 commented Nov 20, 2024

The typos-docker image is around 500MB, it might not be suitable for a test case (It's too slow to run on my local machine).

@bxb100
Copy link
Contributor Author

bxb100 commented Nov 20, 2024

I considered this and added a nextest profile inner .config/nextest.toml to prevent test docker(that does not work with cargo test).

maybe we can preserve it and add #[ignored], only active this on github CI

@bxb100
Copy link
Contributor Author

bxb100 commented Nov 20, 2024

I plan to modify ci-docker.yml to ci-expensive.yml, leverage ignore cfg and nextest filter to disable local test, but work on CI

@j178
Copy link
Owner

j178 commented Nov 20, 2024

(I'll handle the conflicts with master)

@j178
Copy link
Owner

j178 commented Nov 20, 2024

Thanks for your patience, let's merge!

@j178 j178 merged commit f4b0ec9 into j178:master Nov 20, 2024
16 checks passed
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.

Support docker language
2 participants