[Rt-devel] PATCH: Link symmetry

[Ruslan Zakirov]

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, 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.

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


--
Best regards, Ruslan.