pietroppeter / nimib

nimib 🐳 - nim 👑 driven ⛵ publishing ✍
https://pietroppeter.github.io/nimib/
MIT License
181 stars 10 forks source link

Refactor tests and remove hints to better visualize success/failure #202

Closed dlesnoff closed 1 year ago

dlesnoff commented 1 year ago

The for loop on the list of file names is more readable. I added hints:off compilation option, to remove superfluous information for the tests. (Similarly to bigints)

To be consistent in syntax, I can do the same for documentation building.

dlesnoff commented 1 year ago

I did it for docs too. There is an issue with numericalnim which gets in the way when building numerical.nim so I removed it from the list. I mentioned some of the specific dependencies I had to install (due to imports in the documentation) in a comment. It would be nice if this task activates extra requirements so that nimble can do the proper installations automatically.

pietroppeter commented 1 year ago

There is an issue with numericalnim which gets in the way when building numerical.nim so I removed it from the list. I mentioned some of the specific dependencies I had to install (due to imports in the documentation) in a comment. It would be nice if this task activates extra requirements so that nimble can do the proper installations automatically.

Note that there is already a task docdeps that specifies which version of the libraries are needed to build the docs. As such there is no need for an extra comment to mention dependencies, nor I think there is a need to remove numerical from docs.

dlesnoff commented 1 year ago

I reverted last commit since I did not see that extra task. I removed numerical to build other docs, since it stops at the very first build failure.

HugoGranstrom commented 1 year ago

The problem with numerical.nim is that it uses an old version of numericalnim. If you have a more recent version installed it will use that instead and it will fail to compile. I think it is the NumContext[float] that should be NumContext[float, float] for it to work with the latest numericalnim.

dlesnoff commented 1 year ago

I added this fix, even though this could have been in another PR. Do you want me to remove the fix and keep this older version of numerical.nim ? or should the tests be updated to use a more recent version of numericalnim ?

I get an error with penguins.nim:

ggplotnim/postprocess_scales.nim(16, 6) Error: 'getScales' can have side effects

but this should be issued in ggplotnim repository.

pietroppeter commented 1 year ago

I think in this PR we should keep numerical document as it is and in the docs. In another PR we could deal with updating numericalnim and fixing numerical (and others documents such as penguins, to make use of latest versions). With the fix you add now and without changing version of numericalnim in task docdeps the CI fails.

dlesnoff commented 1 year ago

I reverted the numericalnim changes.

pietroppeter commented 1 year ago

Thanks again!

dlesnoff commented 1 year ago

You are welcome!