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

vmath not in nimble requires but imported in windy.nim #78

Closed
creikey opened this issue Mar 18, 2022 · 7 comments
Closed

vmath not in nimble requires but imported in windy.nim #78

creikey opened this issue Mar 18, 2022 · 7 comments

Comments

@creikey
Copy link

creikey commented Mar 18, 2022

No description provided.

@treeform
Copy link
Owner

Thank you for the issue. Windy gets vmath from pixie and its in the requires.

You can see it installs vmath https://github.com/treeform/windy/runs/5541179849?check_suite_focus=true#step:4:23 when using nimble.

Do you have trouble installing or running windy through nimble? Please reply with command that is giving you trouble, so that we can help you.

@creikey
Copy link
Author

creikey commented Mar 18, 2022

Thank you for the issue. Windy gets vmath from pixie and its in the requires.

You can see it installs vmath https://github.com/treeform/windy/runs/5541179849?check_suite_focus=true#step:4:23 when using nimble.

Do you have trouble installing or running windy through nimble? Please reply with command that is giving you trouble, so that we can help you.

More just for completeness since it's imported explicitly in windy here https://github.com/treeform/windy/blob/master/src/windy.nim#L1

@guzba
Copy link
Collaborator

guzba commented Mar 18, 2022

I do not see what including it redundantly in the windy.nimble file actually accomplishes, other than guaranteeing the Windy declared vmath dep and Pixie vmath dep versions fall out of sync in time. Is there a benefit to this completeness? I see costs.

@treeform
Copy link
Owner

If you include vmath in both windy.nimble and pixie.nimble they will get out of sync and its even worse.

I am going to close this issue for now.

If you do nimble install windy or some thing and it does not work, feel free to reopen this issue.

@SolitudeSF
Copy link

How is them getting out of sync worse? What is going to break? Its not a big deal, but i would expect directly used libraries to be specified as deps.

@guzba
Copy link
Collaborator

guzba commented Mar 24, 2022

I asked earlier and still haven't really gotten an answer. If there is some reason we're missing then I'd like to know. Can you please help me understand exactly why this matters? Like in reality, not theoretical things like completeness or expectation. I see no reason to write any line of code (or config) that serves no purpose.

@treeform
Copy link
Owner

In the past we did what you suggested and people had installation issues where the older library was installed ... probably a nimble bug: nim-lang/nimble#890

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

4 participants