cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Bowser <bows...@gmail.com>
Subject Re: [Android] Why is DataResource still in master?
Date Thu, 06 Jun 2013 21:39:50 GMT
I reverted this, now I'm going to recommend that we fork off of
e518eacbd for fixing this.  That being said, things are going to get a
lot uglier post 2.9, so we should probably take a 2.9pre branch and
fix it here with the idea that the plugins have to use this.  Does
that make sense?

On Thu, Jun 6, 2013 at 11:01 AM, Joe Bowser <bowserj@gmail.com> wrote:
> I created a new bug, but yeah, it's the Media AutoTests in MobileSpec.
>  Debugging shows that they're being fed a URI that is missing a /
>
> https://issues.apache.org/jira/browse/CB-3628
>
> On Thu, Jun 6, 2013 at 10:57 AM, Braden Shepherdson <braden@chromium.org> wrote:
>> 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
View raw message