[Webkit-unassigned] [Bug 16401] [GTK] GObject/C DOM binding

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=16401





------- Comment #68 from sam at webkit.org  2008-10-16 13:19 PDT -------
(In reply to comment #67)
> mark, hi,
> 
> all of the issues you raised i have either completed, explained
> the roadblocks and/or reasoning why they have, will or cannot
> be completed, or raised further questions,
> 
> so i - and others - are awaiting responses to those issues.

What issues do you believe are outstanding?  It is unclear.

> 
> the patch as it stands is ... how-to-say... "feature-complete".
> "self-consistent".  "is useable for production-ready purposes".
> "is ready for release".

As Mark noted, the reviewers have deemed that the patch needs further work.  
For instance, there is commented and #ifdefed out code that needs to be
addressed and the changes to the IDL files which are incorrect, both of which
have been pointed out in previous reviews.

> 
> so, _yes_ there's a lot of code ... but... if there's a lot
> of code - tough luck.  it does the job, it's useable, and i'm
> disinclined - and have not yet received any incentive - to
> do additional work to produce what will effectively be non-functional
> and non-useful patches, just so that apple employees can get
> their heads round the work.
> 
> you and others in apple are _paid_ to work on this - you therefore
> have all the time in the world to sit down and work on understanding
> it.

We understand what it does, it simply is not yet up to the standards of the
WebKit project.  We pride ourselves on our code quality and readability and
thus code that simply passes the "functional" test is not necessarily
sufficient for committing.  Our processes for review help us keep this high
standard so we follow them for all code added to the tree.

> 
> issues such as _how_ the code is to be used - such as the interface -
> have _not_ been finalised, and discussions on how the code is to
> be used have not proceeded.  alp raised the issue that the three
> functions which have been added to WebKitWebView are not really
> appropriate an interface, and i agree with him.
> 
> alp suggested *leaving out* those three functions, and i have
> mentioned previously that i agree that that's a very good way
> forward.
> 
> then, for anyone who does want to use this work, they can just
> add in a temporary patch of only a few lines of code, and they're
> off.

Decisions about the API are not relevant to these review comments.

> 
> so, again, we're awaiting responses to the issues.
> 
> i did attempt a "partial approach".  it consumed large amounts
> of my time and disrupted the maintenance of the "real" patch.
> 
> so, if given a choice between whether you, and others at apple, should
> spend time and effort during well-paid working hours, to learn how
> this patch works, or whether i should be asked to "dumb down" what's
> been done into chunks of non-useful work, when i've got £25,000 of
> debt hanging over my head, i'll choose... apple employees doing their
> job and learning something new and exciting in the process!
> 
> 
> the approach that i recommend that you take, due to the size of the
> patch, is to accept it "as-is".  once that has been done, then i will
> be _more_ than happy to raise bugreports and guide others and/or even
> do additional work to address the issues.

This is not an option.  The patch needs to be reviewed, as all patches are,
before they can be landed.  If you would like to expedite this process, you can
take Marks advice and pare down the scope of this patch.

> 
> by removing the (three) function that make it "operational", you
> will be *stopping* the casual developer from assuming that the API
> has been finalised, without hindering the interested developers
> from cleaning up the work.
> 
> there is absolutely no reason why this decision should not be taken,
> and there are many many reasons why it _should_ be taken.
> 
> not least is the simple fact that the sheer number of issues
> that need to be addressed are _far_ too many to keep discussing
> in one bugreport.
> 
> it's simply unmanageable.
> 
> so from a purely _practical_ perspective, accept the damn patch and
> let's get on and make some damn progress.
> 

On a personal note, I wrote the majority of the Objective-C bindings long
before I was employed by Apple.  The approach that I took was to generate a few
core classes to begin with, and incrementally add more generated source files
once the core functionality was in place.  This made the review process nice
and quick due to the small size of the patches.  It also made it easier for me
to address comments as they were made, and learn from them.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.