Re: [ros-diffs] [gdalsnes] 17607: mostly naming changes

[Alex Ionescu]

Hi,

Somewhere in these 14000+ lines, these:

-   PUSER_OBJECT_HDR hdr;

+   PUSER_OBJECT_HDR* ppHdr;

"naming" changes are going to turn into bugs. I can only see 1000 lines 
of your patch, and I can already see it's more then just "naming 
changes". You changed a "pointer" to a "pointer of a pointer". From 
experience, I can tell you that somewhere in those 14000 lines, there's 
a place where you forgot to change the dereference to match the new 
definition. But how can I ever check? The diff isn't posted here, and 
doing it manually would waste my time. And it also shows me that this 
patch probably has a lot more then "naming" changes.

This is as of now, I think the 4th or 5th gigantic patch in this branch with
1) Dubious changes
2) Changes stuck together (naming changes with code changes, etc)
3) Still no changelog.

I am voicing my public disagreement/outcry with the way this branch is 
being handled.

[e-mail] wrote:
> mostly naming changes
>
> typedef struct _USER_REFERENCE_ENTRY
> {
>    SINGLE_LIST_ENTRY Entry;
> PUSER_OBJECT_HDR hdr;
> PUSER_OBJECT_HDR* ppHdr;
>
> } USER_REFERENCE_ENTRY, *PUSER_REFERENCE_ENTRY;
>
> *[truncated at 1000 lines; 13185 more skipped]*

Best regards,
Alex Ionescu

[WaxDragon]

On 9/2/05, Alex Ionescu [e-mail] wrote:
> This is as of now, I think the 4th or 5th gigantic patch in this branch with
> Dubious changes
> Changes stuck together (naming changes with code changes, etc)
> Still no changelog.
>
> I am voicing my public disagreement/outcry with the way this branch is
> being handled.

I've heard a few of the things that this branch supposedly fixes, and
I am interested to see the results.  I also heard that Gunnar is going
to try and break this up into smaller merges.
OTOH, I haven't been able to try the branch yet due to sync issues,
and would really like to see this it tested before it goes in.

WD
-- 
#irc.freenode.net #reactos
01:03PM <filip2307> i don't know about any bug
01:04PM <filip2307> none exist
01:04PM <filip2307> ReactOS is prefect

[Gunnar Dalsnes]

It seems the branch has ended up as a playground/storage for varous 
stuff i wanted to try out, and its likely it has become to messy to 
merge. Therefore ill try to backport/reimpl. (if) any valuable changes 
in trunk.

Its nice to be able to switch between branches, testing things out. 
Maybe we should create an anonymous place in svn, excluded from Sin and 
ros-diffs?

Gunnar

WaxDragon wrote:
> On 9/2/05, Alex Ionescu [e-mail] wrote:
> > This is as of now, I think the 4th or 5th gigantic patch in this branch with
> > Dubious changes
> > Changes stuck together (naming changes with code changes, etc)
> > Still no changelog.
> >
> > I am voicing my public disagreement/outcry with the way this branch is
> > being handled.
>
> I've heard a few of the things that this branch supposedly fixes, and
> I am interested to see the results.  I also heard that Gunnar is going
> to try and break this up into smaller merges.
> OTOH, I haven't been able to try the branch yet due to sync issues,
> and would really like to see this it tested before it goes in.
>
> WD

[Casper Hornstrup]

It sounds like you want SVK: http://svk.elixus.org/

It, however, doesn't change the fact that dumping several
thousand line changes into ReactOS affect the quality of
ReactOS in a negative way. SVK can fortunately keep track
of the individual changes for you.

Casper
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Gunnar Dalsnes
> Sent: 3. september 2005 01:05
> To: ReactOS Development List
> Subject: Re: [ros-dev] Re: [ros-diffs] [gdalsnes] 17607: mostly naming changes
>
> It seems the branch has ended up as a playground/storage for varous
> stuff i wanted to try out, and its likely it has become to messy to
> merge. Therefore ill try to backport/reimpl. (if) any valuable changes
> in trunk.
>
> Its nice to be able to switch between branches, testing things out.
> Maybe we should create an anonymous place in svn, excluded from Sin and
> ros-diffs?
>
> Gunnar

[Casper Hornstrup]

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of Alex Ionescu
> Sent: 3. september 2005 00:07
> To: [e-mail]
> Subject: [ros-dev] Re: [ros-diffs] [gdalsnes] 17607: mostly naming changes
>
> This is as of now, I think the 4th or 5th gigantic patch in this branch with
> Dubious changes
> Changes stuck together (naming changes with code changes, etc)
> Still no changelog.
>
> I am voicing my public disagreement/outcry with the way this branch is
> being handled.

I also have to express my concerns about the extremely large changes made
to this branch. We now build ISO's of each revision. This has the (major)
benefit that we can track down the exact revision that cause a particular
regression without needing to build a lot of ISO's manually. If however,
it turns out that the bug was introduced with a 14.000 line change, you
still have to look at this huge diff to locate the bug. Humans are very
poor at keeping track of this much information so it would require a
considerable amount of time to locate the bug in such a large diff. We
are still very bad at not introducing regressions so we have a lot of
them. At least we should try to develop ReactOS so that the causes of
the regressions are easy to locate and fix.
Assume there is 1 bug per 1000 line change on average. Now this 14.000
line change will contain 14 bugs. How much time is required to locate
the bugs in the diff? Quite a lot, I would imagine. Wouldn't it be more
fun if you could spot the bugs just from reading the diff so you
don't have to waste countless hours of your time with a debugger?
Had it been 14 changes of 1000 lines then this would have been possible
(although 1000 lines is still a lot of information to keep track of,
but it's doable). There would still be 14 bugs, but they would be
spread over several 1000 line changes and some changes might even not
have any bugs, in which case the information to look through is cut
down even further.

Casper