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

Os based lib import path string #23

Merged
merged 1 commit into from
Dec 17, 2021

Conversation

Bandsberg
Copy link
Contributor

Simple fix for #11. Seems a bit hacky. Might not be super robust?

Tested on macOS and Windows 10. @hannobraun will you test on Linux?

Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thanks again, @Bandsberg! Making these fixes and testing on platforms I don't have access to is very appreciated!

I just did a quick test, and it works perfectly on Linux. It would be a bit nicer, if the if expression just returned the file ending, so the format! wouldn't be duplicated for each platform, but this will do for now.

(I do have a rule to keep me sane: It's always okay to write messy code, but it's forbidden to build on top of code that's already messy. This rule has been working exceptionally well for me. Keeps things moving while making sure the mess always stays manageable.)

@hannobraun hannobraun merged commit 6f8b538 into hannobraun:main Dec 17, 2021
@hannobraun
Copy link
Owner

Seems a bit hacky. Might not be super robust?

Oh, I meant to comment on that part, but forgot. I think it's fine. It's an improvement over what was there before, and that's what counts. We can always do better, of course, but that shouldn't prevent us from keeping things moving.

In any case, it works for all the platforms that we seem to have access to. If someone wants to compile this on a BSD or something else, they're welcome to open issues and submit PRs. That why it's open source 😄

@Bandsberg
Copy link
Contributor Author

Bandsberg commented Dec 17, 2021

Happy to help. Feedback on PRs always much appreciated 😄
I might spend a little time making it nicer. I fully agree with your thoughts on continues improvement.

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