search

mjs513 / monoxna

Automatically exported from code.google.com/p/monoxna
Other
0 stars 0 forks source link

patch for model pipeline #4

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
IT add:
- some licence header
- some class for model pipepline
- fix some bug in pipepline
- add with comment some opengl code for vertex/index buffer, effect
- add some nunit test when it is possible but a lot of class can not been
test because of sealed class with internal/private constructor

Original issue reported on code.google.com by olivier....@gmail.com on 18 Feb 2007 at 12:20

Attachments:

GoogleCodeExporter commented 9 years ago 
Hi,

Firstly, that's a lot of code. Good stuff! There are a few problems though.

1) Your patch contains a lot of whitespace changes. I.e. changing four spaces 
into
one tab. That's not good. It makes it really hard for me to see what exactly 
has changed.

2) You have a lot of unfinished things in your code. There are partial
implementations everywhere. Those need to be clearly marked as such. One class
contains a lot of commented out code for example. That can't really be 
committed.

3) There's just too much in that patch. Ideally there'd be one patch for each 
logical
item. i.e. if you implemented the ModelMeshCollection class, it would be in its 
own
patch as it is complete by itself. If you implemented a method which required 
that
you touch 5 other files, then those 5 other files would be included in the diff.

4) That patch is from an old revision of the SVN. You would need to recreate 
your
patch after updating your checkout. If i applied it as it is, it would undo some
changes i made a few days ago.

So, in short. Smaller patches are better. You've done a lot of great work! And 
i'll
commit stuff when it's been nunited (where possible) and comes in a nice small 
(ish)
patch.

If you have any questions, give me a shout.

Thanks.

Original comment by alan.mcg...@gmail.com on 18 Feb 2007 at 2:35

GoogleCodeExporter commented 9 years ago 
Hi,

thanks for feed back:

1) white space issue is because of, at start, someone said to me to switch tab 
to
real tab in vs2005. SO I have done it. I will go back on this and make tab 4 
spaces
as default on vs2005

2) in fact, the model pipline is so big and I am not a pro of opengl. It is why 
when
I see the size I do a patch to have help on it.
I comment all my opengl code to be sure people check it before commit.
How can I mark that a file is partialy implemented?

3) Ok I will try to cut my next patch in few small patch...

4)I have done a svn update before coding anything so you must have code 
something
just after i done my path ;( My patch have been done few days ago. I have just 
commit
it yesterday bu it is older.

Ok thanks for advices, I prefer critik than no answer ;)
I will do my best to done few small patch and fix all space issue.
For sealed class with internal/private constructor what can we do for nunit 
test?

Original comment by olivier....@gmail.com on 18 Feb 2007 at 4:08

GoogleCodeExporter commented 9 years ago 
I have done what I said on issue #5 and #6

Original comment by olivier....@gmail.com on 21 Feb 2007 at 4:40

GoogleCodeExporter commented 9 years ago 
Patch rejected for reasons listed above. The new patches have been dealt with.
Closing this one.

Original comment by alan.mcg...@gmail.com on 21 Feb 2007 at 7:57