Re: Acceptance Tests against a browser (WIP)

From: Atira Odhner <aodhner(at)pivotal(dot)io>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: George Gelashvili <ggelashvili(at)pivotal(dot)io>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Acceptance Tests against a browser (WIP)
Date: 2017-02-09 16:17:40
Message-ID: CA+Vc24qwf1iDiJ9aQDWq1bDHqgsAWbTQ4Y9MkLw+SRhrPtuXgA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave,

I think the problem was that the way you phrased it,

You're right, we totally messed that up. We were talking about making 3
patches and ended up making only 2 and forgot to reword that bit.
Sorry about that.

Here are the two patches for this change that resolves the AttributeError you
were seeing. The first patch is identical to the patch of the same name in
the other email thread.

We're used to
> dealing with larger patchsets via the mailing list - typically as long
> as you're clear about any dependencies, it shouldn't be a problem.
>

Great! We'll try sending patchsets from now on and hopefully that resolves
some of the issues we were seeing.

Tira & George

On Thu, Feb 9, 2017 at 9:28 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> On Thu, Feb 9, 2017 at 2:20 PM, Atira Odhner <aodhner(at)pivotal(dot)io> wrote:
> > Certainly. We did mention the dependency in the email. Would it be
> better
> > to mention it in the patch name?
>
> I think the problem was that the way you phrased it, it sounded
> optional ("an updated patch which does not include adding that test
> helper in case you apply the show-tables patch first"). I think a
> clear "This patch is dependent on patch Foo" would suffice.
>
> > Is there a better way for us to manage
> > these changes? On other open source projects, I've seen github mirrors
> set
> > up so that changes can be pulled in like branches rather then as patch
> > applies. That would have avoided this situation since the parent commit
> > would be pulled in with the same SHA from either pull request branch and
> git
> > would not see it as a conflict.
> >
> > I'm rather new to dealing with patch files like this so I would love some
> > tips.
>
> The Postgres project in general is quite conservative and stuck in
> it's ways about how things are done (which is usually a good thing
> considering you trust your data to the resulting code). We're used to
> dealing with larger patchsets via the mailing list - typically as long
> as you're clear about any dependencies, it shouldn't be a problem.
> Some of us use tools like PyCharms for handling patches and helping
> with reviews etc. which I guess replaces most, if not all of the
> GitHub functionality over plain git.
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Attachment Content-Type Size
0001-Add-create_table-to-test_utils.patch application/octet-stream 1.6 KB
0002-Acceptance-tests-with-input-fix.patch application/octet-stream 17.1 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message George Gelashvili 2017-02-09 17:02:56 Communication Channels
Previous Message Atira Odhner 2017-02-09 15:57:33 Re: [patch] We can see tables in Greenplum!