Code review comment for lp:~gary/juju-gui/bug1090716

crappy network ate my first response, redo with brevity.

On 2013/01/11 13:44:38, gary.poster wrote:
> On 2013/01/11 12:51:03, hazmat wrote:
> > On 2013/01/11 05:29:36, hazmat wrote:
> > > On 2013/01/10 22:08:46, gary.poster wrote:
> > > > Please take a look.
> > >
> > > this seems like a lot more code then just filtering at the model /
delta
> > stream
> > > level.
> >
> >
> > more specifically trying to keep every known view/render location
guarded with
> a
> > service filter is much more fragile and more code then simply
> > removing/preventing in one place at ingest into the app models of
the gui
> > service.

> Benji and I discussed the two options in a pre-implementation call.
We
> preferred filtering on the view level for a few reasons.

> - We were not convinced that it would be any easier in the long run to
make the
> change at the db/ingestion level rather than the view level. If we
had felt
> that this ease was the primary driver for an important choice, we
would have
> considered commissioning competing implementations. However, we had
other
> reasons to not pursue the db/ingestion approach, given below.

> - This is a view-level feature: we don't want to confuse users by
presenting
> certain charms. The db has nothing to do with the desired outcome.
In both of
> our experiences, implementing view-level behavior in view code not
only makes
> more sense intuitively but also is a better long-term design decision.

the purpose of content in the db is for presentation to the user, else
it has no purpose being in the client side db.

the problem with this is a view level feature is that it has to be
implemented everywhere we render a service. If we grow new views we have
to duplicate the same filtering code there as well. From a maintenance
and efficiency perspective it seems a clear win to me, to implement once
vs. implement n.

> - As a concrete example of why keeping the db truthful might be an
advantage, we
> discussed a possible future feature in which hidden services could be
revealed.
> For instance, let's say we want a proxy in front of the gui or some
other future
> hidden service. If we can reveal hidden services with a gesture, we
can make
> this happen, and we can also see why a proxy that appears to be
disconnected is
> actually connected and performing a desired role.

that future use case isn't well defined or even clear that its valid. in
future with jaas, the typical environment won't have these services.

> >
> > the utility functions here feel like they should be in model.utils
instead of
> > view.utils.

> Given our perspective that this is a view-level feature, and that
these utils
> are used in view-level code, the utilities make sense where they are.
Certainly
> if the code is refactored to filter at the ingestion/db level then the
utils
> belong somewhere else.

regardless of usage location, the fact that these only operated on model
attributes (or in future app state) was my differentor to definition
location. its a minor though, implementor's choice on that one.

> >
> > needs fixing, imo.

> I think this is an "implementer's choice" disagreement, where both
sides have
> reasonable arguments and the resolution should be in the hands of the
person who
> did the implementation. That's your call.

i think this one is more an architectural/maintenance question then the
normal implementor's choice. i'd like to do one more round of discussion
before we resolve via other mech.

thanks,

Kapil

https://codereview.appspot.com/7069068/