I toyed with suggesting that you pass env_index to env_detail, and vice
versa, but naah for now.
https://codereview.appspot.com/43860044/diff/1/quickstart/cli/views.py#newcode144
quickstart/cli/views.py:144: envs.get_env_data(env_db, env_name)
Actually getting the data here seems dangerous to me, since each of
these include a "default" flag which is actually determined by the
env_db collectively. As a pattern, I think it would be better to have
env_name only sorted, or possibly env_name mapping to thunks. I haven't
seen whether this actually affects your code yet, but it feels like a
bug magnet.
[Later] Eh, never mind. This is not a long-lived data structure. It's
fine. :-)
I worry a bit that the functional style here is a mismatch with Urwid.
These functions are all about side effects. However, on reflection, I
feel like it's easy to read what you have, and that increasing the
functional feel is increasing the simplicity. We'll see how time and
other people instruct us, I guess. :-)
LGTM with mostly trivial comments (discussion on possible decoys is the
only thing that ended up being interesting).
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/base. py cli/base. py (right):
File quickstart/
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/base. py#newcode37 cli/base. py:37: _Application = namedtuple(
quickstart/
Nice improvement.
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/views. py cli/views. py (right):
File quickstart/
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/views. py#newcode140 cli/views. py:140: detail_view = functools.partial(
quickstart/
nice :-)
I toyed with suggesting that you pass env_index to env_detail, and vice
versa, but naah for now.
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/views. py#newcode144 cli/views. py:144: envs.get_ env_data( env_db, env_name)
quickstart/
Actually getting the data here seems dangerous to me, since each of
these include a "default" flag which is actually determined by the
env_db collectively. As a pattern, I think it would be better to have
env_name only sorted, or possibly env_name mapping to thunks. I haven't
seen whether this actually affects your code yet, but it feels like a
bug magnet.
[Later] Eh, never mind. This is not a long-lived data structure. It's
fine. :-)
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/views. py#newcode180 cli/views. py:180: return env_db, None
quickstart/
this is a decoy, right? No one can use these?
I worry a bit that the functional style here is a mismatch with Urwid.
These functions are all about side effects. However, on reflection, I
feel like it's easy to read what you have, and that increasing the
functional feel is increasing the simplicity. We'll see how time and
other people instruct us, I guess. :-)
https:/ /codereview. appspot. com/43860044/ diff/1/ quickstart/ cli/views. py#newcode244 cli/views. py:244: return env_db, None
quickstart/
Isn't this a decoy? That is, no code will ever actually pay attention
to these results? either index_view will get the mutated result in
set_default, or the urwid caller will get it via use?
https:/ /codereview. appspot. com/43860044/