You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Joe Bowser <bo...@gmail.com> on 2013/05/04 00:47:58 UTC

Pull requests need issues attached to them.

Hey

I just reverted someone's pull request because it broke the nine-patch
splashscreens on Cordova 2.7.  Whenever we accept a pull request, we
should do the following:

 * Make sure that it's actually fixing a bug, and not scratching a
developer's itch
 * Make sure that there's a JIRA issue so that we can re-open the
issue if we have to revert it for some reason
  * Ask on the list whether this actually will break someone's app.
I'm pretty sure that there are some old Nitobi and IBM apps that would
break with this change.

This did teach me a valuable lesson about actually testing for
nine-patch images for multiple orientations, namely that we should
actively do this.  That being said, I think these rules should apply
across the project, and not just with Android.

Thoughts?

Joe

Re: Pull requests need issues attached to them.

Posted by Andrew Grieve <ag...@chromium.org>.
I think that what went wrong in this case is that there was an issue
created, but not referenced by the commit description. I've since made a
habit of re-writing the commit descriptions to include issue numbers.

I'll update the pull request instructions on the wiki (
http://wiki.apache.org/cordova/CommitterWorkflow) to include a note about
re-writing commit messages.


On Mon, May 6, 2013 at 1:57 PM, Lorin Beer <lo...@gmail.com> wrote:

> +1 seems reasonable
>
>
>
> On Sat, May 4, 2013 at 1:49 AM, Tommy-Carlos Williams <tommy@devgeeks.org
> >wrote:
>
> > +1
> >
> > Especially for this part:
> >
> > On 04/05/2013, at 8:47 AM, Joe Bowser <bo...@gmail.com> wrote:
> >
> > > Make sure that there's a JIRA issue so that we can re-open the
> > > issue if we have to revert it for some reason
> >
> >
> >
>

Re: Pull requests need issues attached to them.

Posted by Lorin Beer <lo...@gmail.com>.
+1 seems reasonable



On Sat, May 4, 2013 at 1:49 AM, Tommy-Carlos Williams <to...@devgeeks.org>wrote:

> +1
>
> Especially for this part:
>
> On 04/05/2013, at 8:47 AM, Joe Bowser <bo...@gmail.com> wrote:
>
> > Make sure that there's a JIRA issue so that we can re-open the
> > issue if we have to revert it for some reason
>
>
>

Re: Pull requests need issues attached to them.

Posted by Tommy-Carlos Williams <to...@devgeeks.org>.
+1 

Especially for this part:

On 04/05/2013, at 8:47 AM, Joe Bowser <bo...@gmail.com> wrote:

> Make sure that there's a JIRA issue so that we can re-open the
> issue if we have to revert it for some reason



Re: Pull requests need issues attached to them.

Posted by Filip Maj <fi...@adobe.com>.
That all seems reasonable, ya

On 5/3/13 3:47 PM, "Joe Bowser" <bo...@gmail.com> wrote:

>Hey
>
>I just reverted someone's pull request because it broke the nine-patch
>splashscreens on Cordova 2.7.  Whenever we accept a pull request, we
>should do the following:
>
> * Make sure that it's actually fixing a bug, and not scratching a
>developer's itch
> * Make sure that there's a JIRA issue so that we can re-open the
>issue if we have to revert it for some reason
>  * Ask on the list whether this actually will break someone's app.
>I'm pretty sure that there are some old Nitobi and IBM apps that would
>break with this change.
>
>This did teach me a valuable lesson about actually testing for
>nine-patch images for multiple orientations, namely that we should
>actively do this.  That being said, I think these rules should apply
>across the project, and not just with Android.
>
>Thoughts?
>
>Joe