[OPEN-ILS-DEV] [PATCH] Renewal notice hook, cleanup

[Joe Atzberger]

On Thu, Sep 17, 2009 at 3:20 PM, Dan Scott <dan at coffeecode.net> wrote:

> > Most of the changes are just to formatting via perltidy, moving
> > towards standardized 4-space indents.
>
> I, for one, applaud standardized indents :) Mayhaps you want to add the
> vim comment that sets the standardized settings at a file level to help
> maintain them in future? Throwing the following line at the bottom of
> the file does the trick:
>
> # vim: ts=4:sw=4:noet
>

I'm against that type of annotation, because I find it annoying, there are
plenty of other editors out there, and I don't want to have to put it in
every last file.  But I do plan on submitting a .perltidyrc and .vimrc file
so that, if either were used by developers would accomplish the same thing
from a centralized point of control.


> Also, some other projects that I've worked on have had a rule (informal
> in some cases, more formal in others - hello PHP / PEAR!) that white
> space changes should be committed separately from code changes, to make
> it simpler for other people to focus on the actual code diffs. I don't
> think we have any such rules (informal or formal) about that in
> Evergreen today, but it does make sense to me. Anyone else care to weigh
> in?
>

I think that is a reasonable principle, and it's why I listed the
substantive changes explicitly, so you wouldn't have to scan for them, but I
think separate patches would be fine too, and probably clearer.  (For having
to separate out concurrent edits, I find myself wanting the interactive
rebase and patch splitting capabilities of git.)  However we want to do it
is fine by me.

> The substantive changes are:
> >       * $ses->request('open-ils.trigger.event.autocreate', 'renewal',
> >         $self->circ, $self->circ_lib) if $self->is_renewal;
> >       * 1; added at the end of module,
> >       * SQL line added for renewal hook.
> > Note: also corrected a typo in the SQL for another hook.
>
> One other typo in the same line is "succefully" - might as well clean
> that one up while you're at it.
>

Good catch.  I'll resend with that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20090917/e79554e5/attachment.htm