openshot.developers team mailing list archive

[TJ <ubuntu@xxxxxxxxxxx>]

On Fri, 2009-09-04 at 00:59 -0500, Jonathan Thomas wrote:
> TJ,
> I have spent a few hours tonight reviewing all of the changes.  I am
> working through integrating your branch into the trunk, so please
> don't make any further commits to your branch (for the time being).

No, I wasn't planning on doing any other code work until TRUNK is
ready :)
 
> 
> Here are my initial findings:
> 1) I have no problems with the restructuring of folders / files,
> etc...  Nice Job!
> 2) The biggest issue:  the global variables do not fit with the design
> of OpenShot.  In the spirit of keeping everything object oriented, and
> to fix many issues that global variables create in my local debug
> environment, I've moved your global PATH variables (and a few folder
> checking methods) into the project class.

That sounds sensible. I think I left them as globals since I originally
put them in the launcher, then had some weird issues with the imports
not 'finding' them (probably due to that major 'import' vs 'from'
problem detailed in my r200 commit) and forgot to OO them later.

> 3) There were some indention inconsistanceis.  I've converted all of
> the spaces into tabs, to be consistant with the rest of OpenShot

I found originally that some source files had spaces and some had tabs.
I always prefer spaces and I checked the Python style guide, PEP 8, and
it strongly recommends using spaces.

Once the branches had merged I was going to suggest, as the first thing,
a white-space clean up :)

> 4) There are some PATHs that are being mapped incorrectly, which
> causes some dialogs to not launch

I thought there might be one or two, but since I'd not seen the
application running fully prior to the refactoring I couldn't be sure.


> and lots of regression testing I need to do.  I also have a few more
> changes that I need to integrate into the code (some of the newer
> commits). 

It would make sense to add unit tests to the project as each bit of
functionality is added to ensure code quality to help automate this kind
of assurance in the future.

> Give me another day (time permitting), and hopefully I'll be ready to
> commit these changes to the trunk.

> Thanks for your help TJ!

I'm looking forward to being able to do some 'real' coding on
functionality after this learning curve :)