treeform/windy

vmath not in nimble requires but imported in windy.nim

creikey opened this issue · 7 comments

vmath not in nimble requires but imported in windy.nim

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.

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 commented

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.

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.

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 commented

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.

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