Skip to content

Conversation

@djc
Copy link
Owner

@djc djc commented Apr 7, 2025

No description provided.

@djc djc force-pushed the bindgen branch 5 times, most recently from 965b551 to 3caff3e Compare April 7, 2025 14:29
@djc djc force-pushed the bindgen branch 10 times, most recently from fcf01be to a9f6312 Compare April 7, 2025 16:34
@thomaseizinger
Copy link

Out of curiosity, is the benefit of windows-link that we are decoupling from the particular version of the windows crate? I came here because hostname is the last crate in our dependency tree that still pulls in [email protected] and it would be great to eliminate that.

@djc
Copy link
Owner Author

djc commented Apr 8, 2025

Yup, that's the idea. It also prevents downloading big wads of code that end up largely unused via feature flags...

I've been struggling with getting the codegen lined up correctly, something about the formatting is not working right. Want to take a look?

@thomaseizinger
Copy link

I've been struggling with getting the codegen lined up correctly, something about the formatting is not working right. Want to take a look?

Instead of the current approach, would it make more sense to run the binding generation in CI and then assert that git diff doesn't produce any changes?

@djc
Copy link
Owner Author

djc commented Apr 8, 2025

I've been struggling with getting the codegen lined up correctly, something about the formatting is not working right. Want to take a look?

Instead of the current approach, would it make more sense to run the binding generation in CI and then assert that git diff doesn't produce any changes?

That is kind of the current approach? I've used similar code in a bunch of projects and it mostly works well. But for some reason the formatting isn't sticky here...

@thomaseizinger
Copy link

I've been struggling with getting the codegen lined up correctly, something about the formatting is not working right. Want to take a look?

Instead of the current approach, would it make more sense to run the binding generation in CI and then assert that git diff doesn't produce any changes?

That is kind of the current approach? I've used similar code in a bunch of projects and it mostly works well. But for some reason the formatting isn't sticky here...

Kind of yeah. I was thinking something like:

  • Run binding generation (perhaps still as a test)?
  • Run cargo fmt on the project
  • Run git diff --exit-code

If whitespace is still an issue, we could also consider using --ignore-all-space on git diff to just ignore whitespace altogether.

@thomaseizinger
Copy link

thomaseizinger commented Apr 8, 2025

Unfortunately, CI doesn't run on PRs in this repo so I can't test whether my changes in #30 work as intended.

Edit: I pushed a follow-up commit that enables CI for PRs, needs maintainer approval though.

@djc djc force-pushed the bindgen branch 2 times, most recently from 4f0d6d7 to 591718d Compare April 8, 2025 07:22
@djc
Copy link
Owner Author

djc commented Apr 8, 2025

That feels more like a workaround, though! I think I figured it out, the rustfmt component wasn't installed.

@djc djc force-pushed the bindgen branch 4 times, most recently from 95ad5ff to b552308 Compare April 8, 2025 07:55
@djc djc merged commit 79bf5b4 into main Apr 8, 2025
53 checks passed
@atouchet atouchet mentioned this pull request Sep 17, 2025
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