On 4/10/13, Andrej Golcov <[email protected]> wrote:
>> ... a comment that's also valid for IResourceChangeListener . In Trac
>> , `context` is a variable name associated to instances of
>> `trac.mimeview.api.RenderingContext` (similar to `env` for
>> `trac.env.Environment`) . Therefore it's a bit confusing in this
>> context (at least for me that I'm familiar with that term and how it's
>> used in Trac code base ;) since it clashes with that inherited
>> *convention*. Is there a chance to name these objects somehow
>> different ?
>>
>>   1. evt (dojo)
>>   2. eventObject (jquery)
>>   3. EventArgs (C#)
>>   4. ...
>>
>> If you ask me I prefer (3) or (2) .
> AFIK, "context" parameter is used in multiple places in Trac with
> meaning of context e.g. in resource ResourceContext, in versioncontrol
> in wiki formatters.

That's the point . All those cases make reference to the rendering context .

> IMHO, by renaming the context parameter to
> event(Args|Object) we are losing meaning of this parameter - context
> in wich event is triggered.

It's the same thing ... all C# events are losing semantic information
because of using EventArgs rather than context ?

The fact is also that in Python context is also close to context
managers ... another unrelated concept .

> I will start discussion on Trac dev group
> on previously introduced IResourceChangeListener interface in order to
> get more feedback and see if it can accepted by Trac community.
>

Ok ... I'll add some notes in there to express the same opinions I've
been saying in here ... jftr
;)

>> Just a question . Are these methods supposed to replace manipulators ?
> No. Manipulators events are called before the modification transaction
> is created and can be used for the resource manipulation and
> validation. While IResourceChangingListener events are to be called
> after DB modification is done but before DB transaction is committed.
> These events can be used for additional DB actions that should be
> atomic with the resource DB modification. In our case, it is
> modification of the ticket-relation table atomically with the ticket
> CRUD operation.
>

ok understood .
:)

> I agree with Peter, that the usage of the interface should be very
> clearly documented in order to avoid possible abuse because long
> running transaction can influence the overall system performance.
>

that's what I was worried about . We have three iterations every time
a resource is modified . That will have an impact on performance .

>> Beyond that
>>
>>   - add reparent events
>>   - if `old_values` is considered to be yet another instance of
>>     event arguments then all methods will have the same
>>     interface
>>     * which makes me wander whether they should be
>>       merged into a single method plus event name
>>       included as argument or another field in event args
>>       object (i.e. `context` in signatures above)
>>     * and , by doing so , subsystems would even be
>>       able to define their own event types without
>>       requiring major architectural refactoring .
>
> IMO, using granular interfaces provides documentation by code. While
> more generic events make developer to rely on documentation (that is
> not the strongest part of the open-source) and debugging.
>

I agree with you . But I'd always favor a flexibility architecture
over patching source code . In case a new event is added , or in case
there are particular events for a given resource then :

  1. related components will not be able to
      reuse the notifications mechanism when adding
      their own resource related events
  2. code will have to be modified in order to deal with it

-- 
Regards,

Olemis.

Reply via email to