[ckan-dev] 2939-orgs pull request

Toby Dacre toby.okfn at gmail.com
Fri Oct 12 14:05:17 UTC 2012


On 12 October 2012 14:15, Sean Hammond <sean.hammond at okfn.org> wrote:

> Hi Toby,
>
> I've read through the pull request today:
>
> https://github.com/okfn/ckan/pull/146/
>
> Thanks so much for this work, I think you've done an incredible job to
> produce this especially in the time given. It's a big patch but one
> that is surprisingly easy to understand and produces a surprisingly
> small amount of unease given the size of the patch.
>
>
Thanks

I think David also wants to review it and think he should as you touched
> some scary parts of the code that I don't know much about.
>
>
more eyes the better


> You really removed a lot of code, I like it!
>
> I think the main thing that we need to do before we merge it (or after
> we merge it but before we release 2.0) is at least add some functional
> tests for the new orgs functionality, these could be based on the user
> stories, I feel it's better the ckan team invests the time to do this
> before releasing, rather than releasing and fixing the inevitable bugs
> when users start running into them.
>
>
Again this would be good maybe part of the 'cleaner tests'


> I'd also like to see create_test_data making some test orgs but the ckan
> tests are in need of a big overhaul.
>
> I think that the create_test_data still does too much stuff outside of the
logic actions (sometimes it uses them but sometimes not)  it also calls
them directly not using get_action().  If this was done via actions then
things like org memberships for org creators get setup (and tested) for free

I'll put notes for specific small things in the pull request where we
> can find them later.
>
>
Ones again thanks

> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> http://lists.okfn.org/mailman/listinfo/ckan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.okfn.org/pipermail/ckan-dev/attachments/20121012/ba07df2b/attachment-0001.html>


More information about the ckan-dev mailing list