cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Mocny <mmo...@chromium.org>
Subject Re: Need to revert a CLI breaking change causing CB-5957
Date Wed, 05 Feb 2014 21:03:28 GMT
First off, Jesse I appreciate your respectable tone here, thank you.

I agree, this is a sign that we generally don't test nearly enough on
windows, and should fix that.  As someone who also reviewed the work Mark
was doing here, sorry this wasn't caught.

I'll just add that I think the tests should have been run before the
*tooling release* (and even better, on a regular basis with CI as stated),
not necessarily before every patch to tip of tree lands.  The majority of
changes do not affect specific platforms in subtle ways -- and while we
should absolutely have process to catch those that do -- any process that
involves manually testing in multiple configurations for every single patch
is prohibitive and I think unrealistic.

That change was committed a month ago -- how did we not catch it before
release?

To decrease the odds of this happening again, perhaps we need to amend the
steps for tooling release (
http://wiki.apache.org/cordova/StepsForToolsRelease) to ensure testing on
all the platforms?

-Michal


On Wed, Feb 5, 2014 at 2:34 PM, Jesse <purplecabbage@gmail.com> wrote:

> I would think it would be enough to just make sure that :
> 1. our tests catch the issue
> 2. the tests are run on windows/mac/linux before an npm publish
>
> I agree Mark, the change is valuable, and I don't mean to single you out. I
> am just concerned about how it made it to npm while obviously broken on
> windows devices.
>
> @purplecabbage
> risingj.com
>
>
> On Wed, Feb 5, 2014 at 11:22 AM, Mark Koudritsky <kamrik@google.com>
> wrote:
>
> > Some CI for plugman and CLI on Windows would be extremely useful. I just
> > looked briefly at Travis-CI<
> > http://docs.travis-ci.com/user/getting-started/>,
> > but they only have Linux and OS
> > X<http://docs.travis-ci.com/user/osx-ci-environment/>.
> > Here is a random Windows based service I just found
> > http://www.appveyor.com/,
> > didn't check if it's usable for our case. Of course, this solution would
> > only be for the host side tools, not for on-device tests which are the
> most
> > important ones.
> >
> > That commit was part of this review
> > <https://reviews.apache.org/r/15775/> dealing
> > with CB-4153 <https://issues.apache.org/jira/browse/CB-4153>. But since
> > the
> > patch (probably prepared with git format-patch) contained two separate
> > commits and the second one didn't have a reference to the bug, there is
> no
> > way to deduce that reference. The lesson for me is to add CB-xxxx: prefix
> > to each commit message in a series of related commits. The check was
> added
> > to verity that config.xml does look like it's a Cordova related
> > config.xlmbecause with the new --link-to tag a random file named
> > config.xml by chance could be sitting in that www dir.
> >
> >
> > On Wed, Feb 5, 2014 at 2:14 PM, Steven Gill <stevengill97@gmail.com>
> > wrote:
> >
> > > I'm going to agree with Jesse. That commit should not have made it out
> to
> > > the wild without a platform tag increase. It is fine to go out for 3.4.
> > >
> > > Either we take the commit out and release the CLI again or we revert
> the
> > > CLI to two versions ago (3.3.1-0.2.0) and focus on getting 3.4.0.
> > >
> > > Thoughts?
> > >
> > >
> > > On Wed, Feb 5, 2014 at 10:41 AM, Parashuram Narasimhan (MS OPEN TECH) <
> > > panarasi@microsoft.com> wrote:
> > >
> > > > Is there a way we could have a continuous integration process for the
> > CLI
> > > > too ?
> > > >
> > > > -----Original Message-----
> > > > From: Jesse [mailto:purplecabbage@gmail.com]
> > > > Sent: Wednesday, February 5, 2014 9:54 AM
> > > > To: dev@cordova.apache.org
> > > > Subject: Need to revert a CLI breaking change causing CB-5957
> > > >
> > > > WP8+7 and Windows8 users are currently unable to create new projects
> > > > WP8+with
> > > > the CLI because this commit [1] has shipped.
> > > >
> > > > Here is an issue raised on the subject [2] While I have addressed the
> > > > issue by adding the namespace to the <widget> tag in the platform
> > create
> > > > templates for the affected platforms, until
> > > > 3.4.0 ships this will continue to break.
> > > >
> > > > I am unhappy about how this landed without discussion, or an issue in
> > > > jira, but ultimately this is just a symptom of the fact that not
> enough
> > > > people test on WP7+8 and Windows 8.
> > > > Please try to test all platforms before landing changes to
> cordova-cli,
> > > > cordova-plugman and cordova-js or at least tread lightly and try to
> > aware
> > > > of the impact outside of your pet platforms.  I am always available
> to
> > > > discuss possible impacts.
> > > >
> > > > Cheers,
> > > >   Jesse
> > > >
> > > > [1]
> > > >
> > > >
> > >
> >
> https://github.com/apache/cordova-cli/commit/837e8e367ae4feed4854f9ac95a8e906c893d818
> > > >
> > > > [2] https://issues.apache.org/jira/browse/CB-5957
> > > >
> > > >
> > > > @purplecabbage
> > > > risingj.com
> > > >
> > >
> >
>

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