On 17 Dec 2008, at 05:11, Bill Moseley wrote:

On Tue, Dec 16, 2008 at 06:20:43PM +0000, Tomas Doran wrote:
Do you fancy writing a test for the issue so we can actually prove it is
gone?

Well, it would could be something like this:

<snip>

Again looks perfectly reasonable to me :)

Apologies if I wasn't being clear perviously - could you convert your suggested changes and test into a diff against the distribution which someone could just apply with patch, rather than having to fiddle around copying chunks of code out of email?

But, the plugin also always writes an "expires:" key into the store
(hence the /^session:/ match above), so if the goal is to not write to
the store unless there's something to store then that needs looking
at, too.

Agreed.

If you'd like to include TODO tests for that in the patch (or just fix it, with no TODO in the test), then that would be fantastic.

As you've probably seen from the mailing list already, there are a couple of pending patches on Catalyst-Plugin-Session already:

http://dev.catalyst.perl.org/repos/Catalyst/branches/Catalyst-Plugin- Session/

It's easier to merge patches than chunks of changed code, but I think it's pretty open-season on getting niggles fixed in the Session plugin at the moment. :)

Cheers
t0m

_______________________________________________
List: Catalyst@lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/

Reply via email to