Code review comment for lp:~niemeyer/storm/bug-620615

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

@Jamu,

[1] [2]

Both done, thanks!

@Thomas,

> [1] The value of replace_unknown_lazy doesn't seem to make any tests fail. I
> understand that it's probably tricky to raise this exception, but can you
> explain me why the value is set differently in some cases?

Good point. I've added an additional test doing the counter logic by using
reload (the lazy value should be discarded).

> [2] Previously the check was only done when keep_defined was True, now it's
> only done if keep_defines is False. Shouldn't it be done in both cases?

It is actually done in both cases. Look at how "is_unknown_lazy" is tracked in both branches. The distinction is in how that fact is handled, which is precisely the bug fix.

« Back to merge proposal