cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Braden Shepherdson <bra...@chromium.org>
Subject Re: [Android] Why is DataResource still in master?
Date Thu, 06 Jun 2013 17:57:50 GMT
I'm prepared to have it reverted like it was on the 2.8.x branch. I'll
revert the revert in a branch and try to fix them. I'm working with the app
harness currently and it depends on the DataResource work, but I don't
think it depends crucially on it. Even if it is vital, I could still fix it
up in a branch and use that for our app harness work.

It's not hard to revert it and put it back later, so I think we should go
with that. I'll push it again when it's actually fixed. I agree with you
that the unit tests are a good idea, as well.

Joe, can you give me some test cases where it's failing? Is there a bug? I
don't have a clear sense of the cases where it crashes or does the wrong
thing.

Braden


On Thu, Jun 6, 2013 at 1:45 PM, Joe Bowser <bowserj@gmail.com> wrote:

> I'm OK with it staying in there if:
>
> 1. There's unit tests in the test directory to make sure it works.  This
> should be unit tested
> 2. The unit tests in that directory pass
> 3. All the existing MobileSpec tests pass.
>
> Now, I think that this is going to be tricky, which is why I want it out.
>  However, I don't know what the final decision on the plugins going in on
> 2.9.0 or 3.0.0 is, so I don't know whether we should just suck it up and
> fix it or rip it out yet.  This code seems like it'd work fine for files,
> but not for remote URIs, since that's where we're having the problems.
>
> Is that cool?
>
> On Jun 6, 2013 8:00 AM, "Braden Shepherdson" <braden@chromium.org> wrote:
>
> > Its intention is to provide a single place for URL handling by plugins.
> > Then plugins can capture new schemes, and handle URLs that have been
> > modified by other plugins into URLs they know how to handle. It unifies
> > that various bits of code in different places that knew how to handle
> > gallery URLs and so on.
> >
> > If we want to revert it on master and only push it when we're sure it's
> > working, I can take on the task of rehabilitating it eventually, or it
> can
> > wait until Andrew is back. It isn't vital to the app harness, though it
> > prevents some things from working like app harness-hosted apps accessing
> > gallery URLs.
> >
> > Braden
> >
> >
> > On Wed, Jun 5, 2013 at 11:37 PM, Joe Bowser <bowserj@gmail.com> wrote:
> >
> > > The reverts were only on the 2.8.0 branch, not on master.  It's
> > > currently totally broken right now.
> > >
> > > On Wed, Jun 5, 2013 at 8:09 PM, Michal Mocny <mmocny@chromium.org>
> > wrote:
> > > > 100 yard summary: our intern Shravan from last term was adding this
> as
> > > part
> > > > of his app-harness work.  This specific change landed a too hastily
> as
> > > > there were some issues in corner cases (perhaps over-eagerness due to
> > > time
> > > > pressure as he approach term end), but all actual uses of
> DataResource
> > > > should have been reverted before 2.8 branch (right?), and so just
> idle
> > > code
> > > > remains in the codebase.  The plan is to fix the remaining issues
> > before
> > > > re-adding its usage.. but Andrew was working on that, hence the
> delay.
> > > >
> > > > The specifics details of why it has been added / what its used for, I
> > > will
> > > > defer to some others (Max/Braden?) who would know the answer.
> > > >
> > > > As far as I am aware, leaving it in isn't harmful, but perhaps
> leaving
> > it
> > > > in unfixed in isn't helpful either.  Lets see what Max/Braden say.
> > > >
> > > >
> > > > On Wed, Jun 5, 2013 at 4:54 PM, Joe Bowser <bowserj@gmail.com>
> wrote:
> > > >
> > > >> Hey
> > > >>
> > > >> Why is DataResouce still in master? I don't want this code to go
> into
> > > >> 2.9.0 or 3.0.0, since I have no idea what this is trying to
> > > >> accomplish.  I'm going to start ripping it out of master tomorrow
if
> > > >> someone doesn't tell me why it should still be here.
> > > >>
> > > >> Seriously, WTF?
> > > >>
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message