Hi,<div><br></div><div>A few comments:</div><div><br></div><div>1) there should be empty line between POD and code</div><div><br></div><div>2) The following can be written in one line "my $t = shift || time;":</div>
<div><br></div><div><div><div> ⋅⋅⋅⋅⋅⋅⋅my $t = shift;</div><div> ⋅⋅⋅⋅⋅⋅⋅</div><div> ⋅⋅⋅⋅⋅⋅⋅unless ( $t ) { </div><div> ⋅⋅⋅⋅⋅⋅⋅ ⋅⋅⋅⋅⋅⋅⋅$t = time;</div><div> ⋅⋅⋅⋅⋅⋅⋅}</div></div></div><div><br></div><div>3)  I would prefer 4 spaces rather than tabs, but can convert it before applying</div>
<div><br></div><div>4) all commented code should be deleted before it goes into repository</div><div><br></div><div>5) in the following line $end can be "my $end = $start+24*60*60" rather than function call:</div>
<div><br></div><div>my $end = timelocal_nocheck(59,59,23,@t_array[3..5]);</div><div><br></div><div>6) bday_end has problems if business day spans over midnight</div><div><br></div><div>7) bday_end needs better name without shortings</div>
<div><br></div><div>8) again is_business_day_after and is_business_day suffer from assumption</div><div>that business day can not span over midnight.</div><div><br></div><div>As far as I recall in the latest version we support 24/7 setups with Start => 06:00, End => 06:00. At least I recall doing something about business hours spanning over midnight.</div>
<div><br></div><div>To be included it really need tests.</div><div><br></div><div>Some comments below.</div><div><br><div class="gmail_quote">On Tue, Jul 3, 2012 at 1:21 AM, Alberto Scotto <span dir="ltr"><<a href="mailto:scotto.alberto.86@gmail.com" target="_blank">scotto.alberto.86@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi all,<br><br>I'm not sure this is the right place for discussions about perl modules not part of RT's codebase, but since Business::Hours is strictly related to SLA, and SLA is strictly related to RT.... Here I am.<br>
</blockquote><div><br></div><div>Not a bad place.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Long story short, I added a few subs to Hours.pm in order to have the possibility to set the next business day as the due date. The main sub is <i>end_of_next_business_day</i></blockquote>
<div><br></div><div>I think main sub is very specific for your use case. Instead I would prefer last_after method that is similar to first_after. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
One concern I have is about the timezone. Simply I didn't mind about it, for, as far as I can see, Business::Hours' author didn't mind as well. Is it alright?<br></blockquote><div><br></div><div>The module uses localtime so timezone can be controlled from outside by changing TZ variable and calling POSIX::tzset. There is experimental code in SLA repository to deal with multiple timezones.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Please, test it if you like, and give me some feedback.<br><br>If interested, I can show you how to integrate my custom Hours.pm with   RT::Extension::SLA<br>
<br><br>Regards<span class="HOEnZb"><font color="#888888"><br><br clear="all"><b>Alberto Scotto</b><br>

<img><br>skype:dasgazzo<br><br>
</font></span><br>--------<br>
List info: <a href="http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel" target="_blank">http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Best regards, Ruslan.<br>
</div>