Re: Putting Splinter on bugzilla.gnome.org

On Wed, Oct 07, 2009 at 11:09:35AM -0400, Owen Taylor wrote:
> On Wed, 2009-10-07 at 11:27 +0200, Olav Vitters wrote:
> > On Sun, Oct 04, 2009 at 11:02:36PM -0400, Owen Taylor wrote:
> > >  * A Bugzilla extension for tight integration:
> > > 
> > >    - Links from review comments to the review
> > >    - Links from the attachment table to review pages
> > >    - Posting reviews with attachment status changes doesn't
> > >      send two emails.
> > >    - Integrated appearance (Bugzilla header/footer)
> > 
> > It is an extension that relies on something being installed on the
> > server? How does Splinter connect with Bugzilla? Why have splinter be
> > seperate? Shouldn't everything just stay within the Bugzilla extension?
> 
> Well, two things here:
> 
>  - I don't want Splinter to be bugzilla.gnome.org specific. So it
>    shouldn't be just checked into our bzr repository under extensions/.

I didn't mean that for the Bzr repos. The b.g.o repos should just have a
copy of splinter. Basically, the make install bits. However, I thought
there were other parts that aren't just in the extension.

>  - To keep the (large amounts) of Javascript manageable, it's maintained
>    in separate files then "compiled" into a single file. This means that
>    more than a simple checkout into extensions/ is needed.
> 
> So the way I set it up is that you do a 
> 'make install BUGZILLA_ROOT=/path/to/bugzilla'. 

I don't like that. Just make install it to the b.g.o repos, then commit.
If I update the server I already have to:
bzr up
./checksetup.pl -t
sudo service httpd restart

That the splinter extension is compiled and so on I don't care about if
I 'bzr up'. I don't want e.g. 'bzr up' && 'git pull --rebase' && 'make
install FOO' for b.g.o.

> But it really is structurally just a bugzilla extension (with a ton of
> Javascript code in it.) Or is that way when you run it as an extension -
> there's also a standalone "proxy" mode for development.

Ah ok, I thought it relied on some daemon on the server.

> > What happens if you use an old browser btw? I don't want it to appear if
> > you do not have the right browser. Further, if someone still posts the
> > link, it should fall back properly (e.g. display an warning). At the
> > moment it is very confusing if you see it in IE7.
> 
> Well, depends on the old browser - if your browser just doesn't support
> DOM Storage you just don't get saved drafts - if you leave the page you
> lose your in-progress review.
> 
> Or that's what's supposed to happen. Like any web page, it probably
> doesn't work perfectly cross-browser without some testing and work.
> I'm not completely sure how hard IE7 is going to be - I forget the
> capabilities and limitations there, but it's certainly possible to get
> to work Epiphany/Chrome/IE8/Safari

I don't care if it is cross-browser btw, just that it fails properly
when some unsupported browser is used.

> If you have something old and non-conformant like IE6 the page probably
> just falls apart. The intersection of IE6 users and people who are
> viewing and reviewing patches in GNOME bugzilla is empty.
> 
> I can put some work in on testing and fixing on other browsers than
> Firefox and adding warnings if you are using IE6 ... maybe not
> immediately but soon.

Only just trap the place it fails, then show a warning.

> > >     http://splinter.fishsoup.net/help.html
> > > 
> > > I have more stuff I want to do with it eventually (allow specifying what
> > > lines are being commented, keynav, getting extra context from Git,
> > > tracking of what patches obsolete what, etc), but I think it's useful as
> > > is and what I need most is user feedback.
> > 
> > > My thought at this point is to just to go ahead and install it onto
> > > bugzilla.gnome.org later this week once the GNOME Shell 2.28.0 preview
> > > release is out the door. There's always a chance to break something, but
> > > I think it's pretty small here and I'll be around to fix anything I
> > > break.
> > 
> > We'll need to figure out how to properly handle that extension. This as
> > I want everything in one module, no mix of different things. Perhaps you
> > should commit the extension to the Bzr repos of b.g.o.
> 
> When used as an extension, all the Splinter Javascript/HTML is "part of
> the extension" - there's not really a separate extension that could be
> checked into the b.g.o code.

I was still in the assumption it needed files elsewhere on the server
(something like a daemon).

> > Further, splinter itself should be on git.gnome.org; I saw a typo in a
> > comment in the webservice part which I couldn't fix.
> 
> Yeah, I mentioned moving it in the earlier private mail I sent. I'm
> happy to do that, I just don't want it merged into our Bugzilla bzr
> branch.

Only the 'make install' result, nothing more. I am not proposing you use
Bzr for splinter, just only for b.g.o (I could check it in).

> > btw: Bugzilla HEAD attachment.cgi is a bit nicer.
> 
> If attachment.cgi&action=edit has a text area bigger than 10 rows by 4
> columns, everybody will cheer.

I meant the code, not the UI :-) or :-/

> > I did wonder why you repeated the login method in the webservice part.
> 
> Are you think thinking of the Splinter.info() method? That returns:
> 
>   { 'version': 1,
>     'logged_in': true,
>     'login': [email protected],
>     'name': "Owen Taylor" }
> 
> The version is the Splinter extension API version. The rest of it is
> stuff that there *should* be a way to retrieve in the standard Bugzilla
> RPC methods, but isn't. (Having it here does save a round-trip, but I
> wouldn't have duplicated it if it was available.)

Ah ok. Still, we should enhance the standard RPC stuff though.

> So far I'm trying to make things work with stock Bugzilla 3.4, so I
> don't want to patching the standard webservices and depending on that.
> 
> The call to Bugzilla::login in:
> 
>  my $user = Bugzilla->login;
> 
> Doesn't set new cookies or check the password or anything, it's just the
> standard way to get the login information from the existing cookies sent
> with the request.

Yeah, I quickly read the code, together with things like
check_can_access I wondered if you didn't copy too much. In Bugzilla
HEAD it would be saner.

> > > Yell now if you object.
> > 
> > Seems ok, just wonder about the Splinter <-> Bugzilla interaction. How
> > Splinter retrieves information. I see Splinter->Bugzilla is just
> > XML-RPC, wonder about Bugzilla->Splinter.
> 
> Splinter just uses xmlHttpRequest to fetch:
> 
>  /show_bug.cgi?ctype=xml
>  /attachment.cgi?action=view
> 
> To get bug and attachment data.

Anyway, main thing for me is if you agree with putting the result of
'make install' in Bzr. So not the splinter repository.

-- 
Regards,
Olav