[OH-Dev] Replace controllers with helpers
Rajesh Pradhan
rajeshpradhan at gmail.com
Tue Apr 2 21:27:48 UTC 2013
Hi Asheesh,
Thanks for the tips. I shall try out the mysql tests.
I would love to do similar refactoring to clean up the code. However I
shall get to it next week.
Thanks
Rajesh
On Mon, Apr 1, 2013 at 2:19 PM, Asheesh Laroia <asheesh at asheesh.org> wrote:
> On Sat, 30 Mar 2013, Rajesh Pradhan wrote:
>
> Sorry wrong url in my previous mail for the pull request.
>>
>> https://github.com/openhatch/**oh-mainline/pull/102<https://github.com/openhatch/oh-mainline/pull/102>
>>
>
> #102 looks awesome. I'm testing it more thoroughly now, but on a
> preliminary review it:
>
> * Passes the tests (which is a HUGE hurdle for a massive refactor like
> this)
>
> * Moves the code to the new filenames, rather than just copying it
>
> There are only a few things I would suggest hypothetically changing, if
> you were doing it again:
>
> * Use 'git rebase -i' to 'squash' some of the commits into each other. In
> particular, the commit where you remove controllers.py from the missions
> app (commit log message: "replaced controllers with view_helpers in
> missions app") and the next commit (message: "view_helpers.py added in
> missions app") should be the same.
>
> If they are the same, then git will detect the rename as a rename, rather
> than as a deletion then addition. However, this has little practical
> consequence, so it's not something worth resubmitting over.
>
> Also:
>
> * Add a note to the LAYOUT file indicating what view_helpers.py is for. (I
> can do that myself, once I merge this.)
>
> I'm running it through the MySQL tests, which you yourself can do with:
>
> USE_MYSQL=1 python manage.py test
>
> and comparing that to origin/master now. I expect it to have the same
> amount passing as origin/master (which, sadly, is not 100%), and then I
> can't think of any objections... so I suppose I will deploy it!
>
> (Also, I presumed request 101 was to be ignored now because I saw it is
> "closed" in Github.)
>
> Thank you for this heavy lifting! Once this lands, you've made the
> OpenHatch codebase noticeably simpler to understand.
>
> If refactoring like this is something you enjoy, then I can happily
> suggest another refactoring task! (-: Either way, I hope you keep
> contributing!
>
> -- Asheesh.
>
> ______________________________**_________________
> Devel mailing list
> Devel at lists.openhatch.org
> http://lists.openhatch.org/**mailman/listinfo/devel<http://lists.openhatch.org/mailman/listinfo/devel>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openhatch.org/pipermail/devel/attachments/20130402/084f2a64/attachment.html>
More information about the Devel
mailing list