search

nim-lang / fusion

Fusion is for now an idea about how to grow Nim's ecosystem without the pain points of more traditional approaches.
MIT License
128 stars 16 forks source link

Fix #27 #54

Closed juancarlospaco closed 3 years ago

juancarlospaco commented 3 years ago

https://github.com/nim-lang/fusion/issues/27#issuecomment-744067501

juancarlospaco commented 3 years ago

styleCheck literally found errors on the code, dont you want a more correct code... :disappointed:

alaviss commented 3 years ago

styleCheck literally found errors on the code, dont you want a more correct code... :disappointed:

I don't mind it, but it has no relevance to the objective of this PR, and can be done without the nim action change, so please split it into an another PR.

timotheecour commented 3 years ago

/cc @jiro4989 please advise regarding correct usage of jiro4989/setup-nim-action regarding caching in this PR!

My understanding is:

since we're using builder: windows-2019, not windows-latest

Please remove "adding strictFuncs + styleCheck:error" from this PR. It's not relevant to the issue being addressed.

furthermore, strictFuncs still is very experimental, and it's not at all clear how they can be fixed, see https://github.com/nim-lang/Nim/issues/16305 and https://github.com/nim-lang/Nim/issues?q=is%3Aissue+is%3Aopen+label%3A%22view+types%22+sort%3Aupdated-desc

nimble --experimental:strictFuncs --styleCheck:error test

the problem is this doesn't propagate through exec calls (nim calling nim); instead (in another PR), add tests/config.nims with flags there; it's more flexible (thanks to nimscript) and does propagate through exec calls

jiro4989 commented 3 years ago

@timotheecour

My understanding is:

we need also the snippet mentioned in https://github.com/jiro4989/setup-nim-action#devel---latest-usage we can disregard this note:

Yes.

And setup-nim-action can use Cache nimble on windows-latest and windows-2019 now. I tried those.

choosenim had returned error when nimble had been cached on windows virtual machine. (setup-nim-action is simple wrapper GitHub Actions of choosenim. )

But current choosenim has not this issues.

I will fix this note.

Please should not use Cache nimble on windows-latest. setup-nim-action may Failing to install on windows-latest.

juancarlospaco commented 3 years ago

@alaviss merge?, close?.

timotheecour commented 3 years ago

@xflywind should we merge this ? worst case, we can always revert