[Rt-devel] PATCH: Link symmetry

[Todd Chapman]

On Sat, Nov 05, 2005 at 04:35:19AM +0300, Ruslan Zakirov wrote:
> On 11/5/05, Rolf Grossmann <rg at progtech.net> wrote:
> > Todd Chapman wrote:
> >
> > > OK gang,
> > >
> > > Here is my stab at fixing the asymmetrical link
> > > transaction problem. Added a couple of tests
> > > and everything passes. See any problems?
> >
> > You've been a busy beaver, thank you.
> >
> > I guess you didn't test the other scrips actually not running (the
> > default would be on, right?), because there is a typo in you patch:
> > ActtivateScrips (note the extra t) on both _NewTransaction calls.
> >
> > Also, I usually prefer to have the simpler branch of a condition handled
> > first, especially when it involves a return clause. You've switched that
> > for the one
> >         if ( $args{'Silent'} ) {
> > I guess you wanted to collect the returning at the end of the function.
> IMHO would be better to handle simple branch first and get rid of one
> indent level with
> return bla-bla if $args{'Silent'};
> or
> if( $args{'Silent'} ) {
>   return bla-bla;
> }
> without else block.

OK I just erased a much less friendly reply. Suffice it to say that if
you guys want to code to look different (but be functionally the same) then
you write the code.

> 
> >
> > Ok, now that I'm done nitpicking (sorry for that ;)), I'd also like to
> > remark what you're suggesting now is to always have the second
> > transaction, but to only conditionally run the scrips for it. I don't
> > think that's the right thing to do. I think the argument was to always
> > run scrips with a transaction. If you're being backwards compatible, you
> > can do without the extra transaction aswell. So I'd suggest to
> > completely scrap the extra (misspelled ;)) option and add the condition
> > with the
> >         if ( $remote_uri->IsLocal )
> > like this:
> >         if ( $remote_uri->IsLocal && $RT::LinkMakes2Transactions)
> >
> > What do you think?
> No, you aren't right, "one transaction" is bug and bugs must be fixed,
> if someone poke at old behaviour and used wrong thing(didn't report
> bug) then it's thier problems. IMHO concept of the Todd's patch is ok.

I agree. The bug needs to be fixed.

> 
> >
> > Rolf.
> > _______________________________________________
> > Rt-devel mailing list
> > Rt-devel at lists.bestpractical.com
> > http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel
> >
> 
> 
> --
> Best regards, Ruslan.