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

fix: change windows-only imports to be windows-only #944

Merged
merged 1 commit into from May 16, 2024

Conversation

VlkrS
Copy link
Contributor

@VlkrS VlkrS commented Apr 22, 2024

Apparently this line is required for the Windows build, but leads to warnings on systems that build the eza port with a port of rust 1.77.2, which at this time seems to be limited to the BSDs.

Rather than selectively suppress the warning, I suggest to only include this line where needed.

@fraggerfox
Copy link
Contributor

fraggerfox commented Apr 22, 2024

This fix looks more correct than individually ignoring the BSDs, thank you for the fix.

Out of curiosity, how come this conditional inclusion did not affect linux / macos builds and only BSD builds were having the warning in the first place?

@PThorpe92
Copy link
Member

Thanks for the fix!

Could you edit your commit msg to conform to conventional commits? Then we are all set to merge 👍

PThorpe92
PThorpe92 previously approved these changes Apr 23, 2024
@VlkrS VlkrS changed the title Change Windows-Only Imports to be Windows-Only fix: change windows-only imports to be windows-only Apr 23, 2024
@VlkrS
Copy link
Contributor Author

VlkrS commented Apr 23, 2024

Sure, I hope that one works :-)

@VlkrS
Copy link
Contributor Author

VlkrS commented Apr 23, 2024

This fix looks more correct than individually ignoring the BSDs, thank you for the fix.

Out of curiosity, how come this conditional inclusion did not affect linux / macos builds and only BSD builds were having the warning in the first place?

I assume it's because Linux and MacOS builds were using the rust version from Cargo.toml (1.70.0) and only the BSD ports were at 1.77.2 already...

Copy link
Member

@cafkafk cafkafk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the merge commit a024295. normally, we only update branches with rebase.

Apparently this line is required for the Windows build, but leads to warnings on systems that build the eza port with a port of rust 1.77.2, which at this time seems to be limited to the BSDs.

Signed-off-by: Volker Schlecht <47375452+VlkrS@users.noreply.github.com>
@cafkafk cafkafk merged commit e8e0b6d into eza-community:main May 16, 2024
19 checks passed
@cafkafk
Copy link
Member

cafkafk commented May 16, 2024

Thanks for the contribution! 🎉

@VlkrS VlkrS deleted the unused-imports branch May 16, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants