refactor: Explicitly add proj_root to FlowCfg#150
refactor: Explicitly add proj_root to FlowCfg#150rswarbrick wants to merge 1 commit intolowRISC:masterfrom
Conversation
There was a problem hiding this comment.
This looks reasonable to me, but it's kind of a change to please the type checker instead of a change to fix the issue the type checker is telling us about. The useful aspect of the previous behaviour that is now gone is that a missing proj_root would not give cryptic errors. Perhaps whilst everything is still not cleanly defined we should add an if not self.proj_root: raise ValueError(...) statement right after calling self._expand in FlowCfg.__init__ just to avoid this potential head-scratcher?
Note also that in #139 (merged earlier today), I made an explicit reference to self.proj_root on the base flow anyway, right at the point I am mentioning.
Hit the wrong button, meant to be a comment.
This gets loaded up from the hjson file (and nothing will work if it's missing). This commit tweaks things so that we check the field has been loaded. This will always have type str (because it gets loaded directly from an hjson file). Add an explicit cast to Path when calling the WorkspaceConfig constructor. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
0082fcd to
30814c6
Compare
|
Thanks for the review, @AlexJones0. Does the check look more reasonable now? |
This gets loaded up from the hjson file (and nothing will work if it's missing). This commit tweaks things so that we check the field has been loaded.
This will always have type str (because it gets loaded directly from an hjson file). Add an explicit cast to Path when calling the WorkspaceConfig constructor.