public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH] Document that a patch should be submitted to the list as an attachment
6+ messages / 2 participants
[nested] [flat]

* [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-10 18:40  Chris Mayo <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Chris Mayo @ 2016-10-10 18:40 UTC (permalink / raw)
  To: pgadmin-hackers

Signed-off-by: Chris Mayo <[email protected]>
---
 docs/en_US/submitting_patches.rst | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)



-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers


Attachments:

  [text/x-patch] 0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch (998B, 2-0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch)
  download | inline diff:
diff --git a/docs/en_US/submitting_patches.rst b/docs/en_US/submitting_patches.rst
index c077551..4ac878d 100644
--- a/docs/en_US/submitting_patches.rst
+++ b/docs/en_US/submitting_patches.rst
@@ -30,7 +30,7 @@ to create a patch between your development branch and the public master branch.
 
 Once you have your patch, check it thoroughly to ensure it meets the pgAdmin
 :doc:`coding_standards`, and review it against the :doc:`code_review` to minimise
-the chances of it being rejected. Once you're happy with your work, mail it to the 
-mailing list [email protected]. Please ensure you include a full 
-description of what the patch does, as well as the rationale for any important 
-design decisions.
+the chances of it being rejected. Once you're happy with your work, mail it
+as an attachment to the mailing list [email protected].
+Please ensure you include a full description of what the patch does,
+as well as the rationale for any important design decisions.


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-11 14:34  Dave Page <[email protected]>
  parent: Chris Mayo <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Dave Page @ 2016-10-11 14:34 UTC (permalink / raw)
  To: Chris Mayo <[email protected]>; +Cc: pgadmin-hackers

Hi

On Monday, October 10, 2016, Chris Mayo <[email protected]> wrote:

> Signed-off-by: Chris Mayo <[email protected] <javascript:;>>
> ---
>  docs/en_US/submitting_patches.rst | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
>
This doesn't apply - can you rebase it please?

Thanks.

(pgadmin4)snake:pgadmin4 dpage$ git apply
~/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch
/Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:13:
trailing whitespace.
the chances of it being rejected. Once you're happy with your work, mail it
/Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:14:
trailing whitespace.
as an attachment to the mailing list [email protected].
/Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:15:
trailing whitespace.
Please ensure you include a full description of what the patch does,
/Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:16:
trailing whitespace.
as well as the rationale for any important design decisions.
error: patch failed: docs/en_US/submitting_patches.rst:30
error: docs/en_US/submitting_patches.rst: patch does not apply



-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-11 17:14  Chris Mayo <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Chris Mayo @ 2016-10-11 17:14 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: pgadmin-hackers

On 11/10/16 15:34, Dave Page wrote:
> Hi
> 
> On Monday, October 10, 2016, Chris Mayo <[email protected] <mailto:[email protected]>> wrote:
> 
>     Signed-off-by: Chris Mayo <[email protected] <javascript:;>>
>     ---
>      docs/en_US/submitting_patches.rst | 8 ++++----
>      1 file changed, 4 insertions(+), 4 deletions(-)
> 
> 
> This doesn't apply - can you rebase it please? 
> 
> Thanks.
> 
> (pgadmin4)snake:pgadmin4 dpage$ git apply ~/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch 
> /Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:13: trailing whitespace.
> the chances of it being rejected. Once you're happy with your work, mail it
> /Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:14: trailing whitespace.
> as an attachment to the mailing list [email protected] <mailto:[email protected]>.
> /Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:15: trailing whitespace.
> Please ensure you include a full description of what the patch does,
> /Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:16: trailing whitespace.
> as well as the rationale for any important design decisions.
> error: patch failed: docs/en_US/submitting_patches.rst:30
> error: docs/en_US/submitting_patches.rst: patch does not apply
> 

I checked and I can't see anything wrong. I also tried downloading the patch from the web archive and using git apply.

The "trailing whitespace" errors may suggest the problem - the added lines in the patch don't have any whitespace.
Have you been near a Windows machine? I used unix2dos on the patch and then got the same result as above.

Chris



-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-11 21:03  Dave Page <[email protected]>
  parent: Chris Mayo <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Dave Page @ 2016-10-11 21:03 UTC (permalink / raw)
  To: Chris Mayo <[email protected]>; +Cc: pgadmin-hackers

Hi

On Tuesday, October 11, 2016, Chris Mayo <[email protected]> wrote:

> On 11/10/16 15:34, Dave Page wrote:
> > Hi
> >
> > On Monday, October 10, 2016, Chris Mayo <[email protected]
> <javascript:;> <mailto:[email protected] <javascript:;>>> wrote:
> >
> >     Signed-off-by: Chris Mayo <[email protected] <javascript:;>
> <javascript:;>>
> >     ---
> >      docs/en_US/submitting_patches.rst | 8 ++++----
> >      1 file changed, 4 insertions(+), 4 deletions(-)
> >
> >
> > This doesn't apply - can you rebase it please?
> >
> > Thanks.
> >
> > (pgadmin4)snake:pgadmin4 dpage$ git apply ~/Downloads/0001-Document-
> that-a-patch-should-be-submitted-to-the-lis.patch
> > /Users/dpage/Downloads/0001-Document-that-a-patch-should-
> be-submitted-to-the-lis.patch:13: trailing whitespace.
> > the chances of it being rejected. Once you're happy with your work, mail
> it
> > /Users/dpage/Downloads/0001-Document-that-a-patch-should-
> be-submitted-to-the-lis.patch:14: trailing whitespace.
> > as an attachment to the mailing list [email protected]
> <javascript:;> <mailto:[email protected] <javascript:;>>.
> > /Users/dpage/Downloads/0001-Document-that-a-patch-should-
> be-submitted-to-the-lis.patch:15: trailing whitespace.
> > Please ensure you include a full description of what the patch does,
> > /Users/dpage/Downloads/0001-Document-that-a-patch-should-
> be-submitted-to-the-lis.patch:16: trailing whitespace.
> > as well as the rationale for any important design decisions.
> > error: patch failed: docs/en_US/submitting_patches.rst:30
> > error: docs/en_US/submitting_patches.rst: patch does not apply
> >
>
> I checked and I can't see anything wrong. I also tried downloading the
> patch from the web archive and using git apply.
>
> The "trailing whitespace" errors may suggest the problem - the added lines
> in the patch don't have any whitespace.
> Have you been near a Windows machine? I used unix2dos on the patch and
> then got the same result as above.
>

No, I've only been on my Mac. Your suggestion made me think though - I
opened the patch in my editor, and it claims the original had Windows line
endings. I converted them to \n and it applied just fine.

What does 'git config core.autocrlf' output for you?

In any case, I applied the patch. Thanks!


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-12 11:33  Chris Mayo <[email protected]>
  parent: Dave Page <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Chris Mayo @ 2016-10-12 11:33 UTC (permalink / raw)
  To: Dave Page <[email protected]>; +Cc: pgadmin-hackers

>     > /Users/dpage/Downloads/0001-Document-that-a-patch-should-be-submitted-to-the-lis.patch:16: trailing whitespace.
>     > as well as the rationale for any important design decisions.
>     > error: patch failed: docs/en_US/submitting_patches.rst:30
>     > error: docs/en_US/submitting_patches.rst: patch does not apply
>     >
> 
>     I checked and I can't see anything wrong. I also tried downloading the patch from the web archive and using git apply.
> 
>     The "trailing whitespace" errors may suggest the problem - the added lines in the patch don't have any whitespace.
>     Have you been near a Windows machine? I used unix2dos on the patch and then got the same result as above.
> 
> 
> No, I've only been on my Mac. Your suggestion made me think though - I opened the patch in my editor, and it claims the original had Windows line endings. I converted them to \n and it applied just fine.

I think it's an email thing. From:
https://github.com/git/git/commit/8d8140843501107c92e2f9a5acb60ee136352c1f
"The problem is that SMTP transport is CRLF-unsafe.  Sending a patch by
email is the same as passing it through "dos2unix | unix2dos".  The newly
introduced CRLFs are normally transparent because git-am strips them."

I have been using git send-email, to avoid the risk of a mail client mangling patches.
But I can try using Thunderbird in future.

I guess git imap-send behaviour is similar:
http://www.spinics.net/lists/git/msg160136.html
"
> I'm using git imap-send to send patches to wine-patches, and it seems
> like it converts all my patches to have CRLF line endings?

The canonical line ending for mail is CRLF. So yes, it will convert your
patch to CRLF for storage. But anything pulling it out of the IMAP
folder should convert it back to native line endings."


Testing locally the raw emails do have CRLF saved from Thunderbird as .eml,
although when I save just the attachment the patch only has LF.

Not sure why this hasn't been an issue before on the list.

The Git tools seem to be more focussed on inline messages. On the receiving end 
using attachments the ideas I have:
 - Is there a save as text option in your client?
 - Save from the web archive

> 
> What does 'git config core.autocrlf' output for you?

The only core. attribute I have set is core.editor.

> 
> In any case, I applied the patch. Thanks!
> 

Hopefully we don't have to add more instructions as a result of this discussion...

Chris



-- 
Sent via pgadmin-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: [PATCH] Document that a patch should be submitted to the list as an attachment
@ 2016-10-12 14:06  Dave Page <[email protected]>
  parent: Chris Mayo <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Dave Page @ 2016-10-12 14:06 UTC (permalink / raw)
  To: Chris Mayo <[email protected]>; +Cc: pgadmin-hackers

Hi

On Wednesday, October 12, 2016, Chris Mayo <[email protected]> wrote:

> >     > /Users/dpage/Downloads/0001-Document-that-a-patch-should-
> be-submitted-to-the-lis.patch:16: trailing whitespace.
> >     > as well as the rationale for any important design decisions.
> >     > error: patch failed: docs/en_US/submitting_patches.rst:30
> >     > error: docs/en_US/submitting_patches.rst: patch does not apply
> >     >
> >
> >     I checked and I can't see anything wrong. I also tried downloading
> the patch from the web archive and using git apply.
> >
> >     The "trailing whitespace" errors may suggest the problem - the added
> lines in the patch don't have any whitespace.
> >     Have you been near a Windows machine? I used unix2dos on the patch
> and then got the same result as above.
> >
> >
> > No, I've only been on my Mac. Your suggestion made me think though - I
> opened the patch in my editor, and it claims the original had Windows line
> endings. I converted them to \n and it applied just fine.
>
> I think it's an email thing. From:
> https://github.com/git/git/commit/8d8140843501107c92e2f9a5acb60ee136352c1f
> "The problem is that SMTP transport is CRLF-unsafe.  Sending a patch by
> email is the same as passing it through "dos2unix | unix2dos".  The newly
> introduced CRLFs are normally transparent because git-am strips them."
>
> I have been using git send-email, to avoid the risk of a mail client
> mangling patches.
> But I can try using Thunderbird in future.
>
> I guess git imap-send behaviour is similar:
> http://www.spinics.net/lists/git/msg160136.html
> "
> > I'm using git imap-send to send patches to wine-patches, and it seems
> > like it converts all my patches to have CRLF line endings?
>
> The canonical line ending for mail is CRLF. So yes, it will convert your
> patch to CRLF for storage. But anything pulling it out of the IMAP
> folder should convert it back to native line endings."
>
>
> Testing locally the raw emails do have CRLF saved from Thunderbird as .eml,
> although when I save just the attachment the patch only has LF.
>
> Not sure why this hasn't been an issue before on the list.
>

It has, on rare occasions. One particular friend of mine had to resort to
gzipping patches for pgAdmin 4, but it's been a non-issue for pretty much
everyone else here - and the process we use is pretty standard on the
PostgreSQL lists as well, where it's also not generally a problem.

Note that at least for those of us at EDB, Gmail is the standard client we
use. I wonder if some combination of sending and receiving MUAs is what
causes the problem.


>
> The Git tools seem to be more focussed on inline messages. On the
> receiving end
> using attachments the ideas I have:
>  - Is there a save as text option in your client?
>

You can view the original message in Gmail, but that's the entire raw text
of it, so you get all the mime parts and headers as well.


>  - Save from the web archive
>

That's a possibility, though it's such a rare issue that I doubt we'll
remember to do it.

I'd really like to know the root cause - it's quite annoying not being able
to pin it down.


>
> >
> > What does 'git config core.autocrlf' output for you?
>
> The only core. attribute I have set is core.editor.
>
> >
> > In any case, I applied the patch. Thanks!
> >
>
> Hopefully we don't have to add more instructions as a result of this
> discussion...
>

Well at least it's keeping you busy :-)

Thanks, Dave.


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2016-10-12 14:06 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-10-10 18:40 [PATCH] Document that a patch should be submitted to the list as an attachment Chris Mayo <[email protected]>
2016-10-11 14:34 ` Dave Page <[email protected]>
2016-10-11 17:14   ` Chris Mayo <[email protected]>
2016-10-11 21:03     ` Dave Page <[email protected]>
2016-10-12 11:33       ` Chris Mayo <[email protected]>
2016-10-12 14:06         ` Dave Page <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox