[jifty-devel] Patches via irc

[Jesse Vincent]

On Wed, Apr 18, 2007 at 10:29:04PM -0700, Marc Mims wrote:
> Jesse,
> 
> I'm new to Jifty, working on a project where the client requested Jifty
> as the framework.  So I'm trying to get up to speed as quickly as I can.

Hi,

Welcome!


> I'm semifor on irc and nopasted a couple of patches there.

And they're much appreciated. But we're not a super-irc focused
community, which means that there's a chance we could miss your patches.
And that would suck.

> Your preference for using the e-mail list works fine for me.  I hadn't
> yet joined the list and had the patches not gotten a response I would
> have joined and posted them.

No worries at all. 

> I've worked in other projects where the common practice is to post
> trivial patches to the irc channel for a quick once over by any devs
> that happen to be online.  Sometimes they say "take it to the list",
> sometimes they patch them directly, and often they offer guidance on how
> to improve the patch.

Understood. And I suspect that if they were in the context of a
conversation, it wouldn't have even occurred to me to mention the
preference for sending them via the list.

> I was just testing the water.  I'm happy to work with the local

> The patches I provided were for issues I discovered while trying to get
> the trunk version working with the Doxory example.

*nod* 

> The patch you didn't apply is trivial and perhaps not even worth
> mentioning.  You indicated it was a noop.  It may not be important, but
> it isn't exactly a noop.  The current code will never produce the value
> 255.  It will produce values 0 through 254 inclusive.  My patch produces
> values 0 through 255 inclusive.  If the intent was to include 255 in the
> values produced, then my patch is a fix.  If not, it's immaterial.

The goal of the code you patched is to end up with relatively random
data. I was fairly sleepy from a long day on-site at a customer and an 8
hour flight afterward, but I _think_ that what I meant was that it was a
semantic noop in that it wouldn't change the system's overall behaviour,
not that it wouldn't generate differerent randomness ;)

> 
> I've included the patch here, again, for your review.  No offense taken
> if it isn't applied.  Like I said, it is trivial even if it is a "fix".

I don't have any objection to the patch..and it'd make a good first
test of a shiny new commit bit ;) Mail me the output of 'htpasswd -n'
off-list and I'll generate you a commit bit.

Best,
Jesse

> 	-Marc
> 
> Index: lib/Jifty/Plugin/Authentication/Password/Mixin/Model/User.pm
> ===================================================================
> --- lib/Jifty/Plugin/Authentication/Password/Mixin/Model/User.pm        (revision 3154)
> +++ lib/Jifty/Plugin/Authentication/Password/Mixin/Model/User.pm        (working copy)
> @@ -128,7 +128,7 @@
>      my $self = shift;
>      my $auth_token = '';
>  
> -    $auth_token .= unpack('H2', chr(int rand(255))) for (1..16);
> +    $auth_token .= unpack('H2', chr(int rand(256))) for (1..16);
>  
>      $self->__set(column => 'auth_token', value => $auth_token);
>  }
> 
> _______________________________________________
> jifty-devel mailing list
> jifty-devel at lists.jifty.org
> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-devel
> 

--