Makes ...there_is_another | false | okay in tables
#580
Conversation
BryceStevenWilley
added
question
There was a problem hiding this comment.
This is permissive, but adds branching complexity that may not be necessary. Adding a test can help with that. If this is implemented, I'd rather leave it out of the documentation to avoid user confusion. That way, if it happens accidentally (e.g. with the Story Table generator), it's not a blocker, but it avoids people trying to explicitly use it.
|
The changelog comment should mention that this should not be documented [and why it's not being documented]. |
Sets up a valid situation where you would need `there_is_another` to be directly set to false, instead of using `target_number` like the docs say. First used in SuffolkLITLab/ALKiln#580
Sets up a valid situation where you would need `there_is_another` to be directly set to false, instead of using `target_number` like the docs say. First used in SuffolkLITLab/ALKiln#580
BryceStevenWilley
force-pushed
the
there_is_another_false
branch
from
919340a
to
72228f1
Compare
|
Sorry to waffle on this, but regarding documentation we might want to have another conversation. I'm now rethinking. It might be ok to document with a hefty warning. Otherwise I'm concerned we'll forget this and be surprised by it later. Now that I look at it again, the case you made for the need is compelling. |
I'm not super concerned about this, given
Still okay with adding this in the documentation though. My concern isn't that it needs a warning (IMO it doesn't), but that it's adding complexity to our main suggested solution, which is "just use target_number". |
|
Fair enough. Let's just leave it out for now in that case. |
`there_is_another | true` is invalid in a story table, because it will always be true, and start an infinite loop of adding more elements to whatever list is gathering. However, you might be asked to add elements to a pre-filled list. In my case, I add Individuals to the `users` and `other_parties` from existing EFM case information, and then ask if the interviewee wants to add more users. Therefore I can't use `target_number` in the story table, as the number won't include the number of elements already added to the list (which Kiln can't figure out). I can however, use `there_is_another = False`, which will terminate.
Ensure that no warnings are thrown in the report, and that it can successfully pass. Needed to add functions to check for the non-existance of things in the report, and corrected some misspellings in other feature files that I came across.
BryceStevenWilley
force-pushed
the
there_is_another_false
branch
from
72228f1
to
fcc9e8a
Compare
|
If I'm reading your last comment correctly, you agree that this doesn't need extra documentation outside of the CHANGELOG, and this is ready for merging, correct? There is a test added for it, and it's in the CHANGELOG now, so I think everything else is ready. |
there_is_another | trueis invalid in a story table, because it will always betrue, and it will start an infinite loop of adding more elements to whatever list is
gathering.
However, you might be asked to add elements to a pre-filled list. In my case, I
add Individuals to the
usersandother_partiesfrom existing EFM caseinformation, and then ask if the interviewee wants to add more users. Therefore
I can't use
target_numberin the story table, as the number won't include thenumber of elements already added to the list (which Kiln can't figure out). I
can however, use
there_is_another = False, which will terminate.This does change how things are handled and documented, but IMO is a good
change. Let me know if we want a discussion here.