Closed HugoGranstrom closed 1 year ago
If CI is green now, I'm content.
@pietroppeter want to have a look?
Looking at it from mobile so putting comments here:
if I understand right the Nim cache folder is original folder + directory in which source resides + folder with name of file without extension? Is it normal/necessary to have name of file on the directory? Or it is a choice for other reasons? I am fine with it just wanted to understand
Correct! The reason I went with the name of the file is to make the structure cleaner and prevent any unforeseen file name clashes in the future. Now that all files get their own directory, they are totally isolated from each other.
maybe strformat is overkill for what appears to be concatenation of two strings... ;)
:hand_over_mouth:
I think some comment to reminds us why we are doing this cache thing could be useful (avoid conflicts on parallel builds, right?)
Good idea, will add :+1:
this should also be mentioned in the documentation
Agree, but where? Create a new bookpage for Parallel Builds
?
I imagine at some point we might want to be able to override (e.g. if nimcache is provided in options...), but I guess it is not worth doing it now
I thought about this as well. I don't see why we would want to override it for nimiBooks though. But I guess we could just play with the order of our --nimcache:
and the nimOptions
. I don't know if it is the first or last occurrence that takes precedence, but it should be easy to make it overridable.
On documentation, in this page there is a section about parallel builds, we could add a paragraph there about this custom Nim cache (explaining the index example).
For overriding you are right, maybe it is even already available (my guess would be the last parameters would override). But on second thoughts I think it might be very rare to have this needs of overriding (you would not want to override Nim cache with a single folder for all parallel builds), and if you really do, well you can always write your custom Nim commands for that, nimibook itself is not really doing anything special. So I would vote to keep it like it is now.
On documentation, in this page there is a section about parallel builds, we could add a paragraph there about this custom Nim cache (explaining the index example).
Added! :+1:
For overriding you are right, maybe it is even already available (my guess would be the last parameters would override). But on second thoughts I think it might be very rare to have this needs of overriding (you would not want to override Nim cache with a single folder for all parallel builds), and if you really do, well you can always write your custom Nim commands for that, nimibook itself is not really doing anything special. So I would vote to keep it like it is now.
Indeed, it's not something we would have to handle. If the user has the need for it, then they are already deep in some shit and can probably handle themselves :wink:
Any input on the added documentation?
On the documentation, I would use as example two index files (e.g. index.nim
and tutorials\index.nim
) instead of tut1.nim
, similar to what you have in the comment.
Good point, fixed!
Nice! :) I don't have merge privileges in this repo so you'll have to merge this.
Will need to fix that too! ;)
We'll fix it at the same time as we move it to nimib-land 🤠jokes aside, I should transfer nimiBoost some day.
Hopefully before but fair game 😂
fixes #81
This PR changes the nimcache folder such that
.nim
files in different folders get different nimcache folder.I'm not entirely done, but I want to see what the CI says.