-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Currently math.margins is installed in a new vocabulary root "pre", which is specifically given priority loading during imaging. There are a couple problems with this, listed below. math.margins should be installed following regular factor practice, in the "work" root (same place as testest is installed).
The problems:
math.marginsdoes not need to prioritised loading, doing so adds unnecessary complexitymath.marginsbeing in a prioritised root violates the factor principle of earlier loaded roots not relying on vocabs from later loaded roots. vocabs incoredo not rely on vocabs inbasisdo not rely on vocabs inextra.math.marginshowever imports vocabs fromcoreandbasis, but is loaded before them.math.marginsbeing prioritised means that if amath.marginsvocabulary is added to the standard library in the future, then any vocabs in the standard library which rely on it will break, since they will load the externalmath.marginsinstead. Causing the standard lib to break internally, is not a good thing.- Additionally, if a standard library
math.marginsvocabulary is added but no other vocabulary relies on it, then no error will be caused during docker image creation, but it will be silently shadowed. This means that any users that try to use it in solutions will instead import the externalmath.marginsinstead of the expected standard library version. - Working around the standard factor library structure and load order is a hack and bad practice, precisely because of the bugs it could introduce
In summary, the current behaviour is bad practice, and is actively creating potential future compatibility issues. On top of that, it is unnecessary - math.margins will work exactly the same from "work" as it does from "pre".
If the intent in this design is to provide an "early warning system" for future versions to inform that there is a name conflict, then adding an explicit test for that is a clearly better option, rather than relying on the standard lib potentially breaking during compilation.