Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Michael Eager <eager@eagerm.com>
To: Andrew Guirguis <andrew.sadek.se@gmail.com>
Cc: gcc-patches@gcc.gnu.org, gdb-patches@sourceware.org,
	binutils@sourceware.org
Subject: Re: [PATCH] [Microblaze]: PIC Data Text Relative
Date: Tue, 27 Feb 2018 00:07:00 -0000	[thread overview]
Message-ID: <3ea62194-f081-0d2e-13f4-5884f1615e37@eagerm.com> (raw)
In-Reply-To: <CAE=jbAMyXXpWUPnC_dcmyWNeOXcJ-Wo4+KomQ9KwpX2KuLDqkg@mail.gmail.com>

On 02/25/2018 11:44 PM, Andrew Guirguis wrote:
> Dears,
> 
> Kindly find attached the patch bundle for Microblaze 
> '-mpic-data-text-relative' feature.
> 
> Description of the feature in the following link:
> https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md 
> <https://github.com/andrewsadek/microblaze-pic-data-text-rel/blob/pic_data_text_rel/README.md>
> 
> Bundle includes:
> 1) Change logs for GCC, binutils
> 2) GCC Test results and comparison with the original.
> 3) New Test case (picdtr.c)
> 4) The Patches (against current heads)

Hi Andrew --

Thanks for the submission.  I have the following recommendations:

Submit each patch to the appropriate project mailing list.  Only submit
the patch for the specific project, without patches for other projects.

Include a description of the changes with each patch as well as the
changelog.  Include the patch in your email or as an attachment.

It isn't clear why you sent your submission to the gdb-patches mailing
list, since there don't appear to be any GDB changes.  Conversely, it is
not clear why you did not include the binutils mailing list, since you
include a patch to that project.

Be sure to follow GNU coding conventions,  Check brace placement,
indent, maximum line length, if statements, etc.  I noticed a number
of places where these conventions are not followed in your patches.

GCC regression tests should include all tests (e.g., gcc.dg), not just 
the limited number of MicroBlaze-specific tests.

-- 
Michael Eager    eager@eagerm.com
1960 Park Blvd., Palo Alto, CA 94306


       reply	other threads:[~2018-02-27  0:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAE=jbAMyXXpWUPnC_dcmyWNeOXcJ-Wo4+KomQ9KwpX2KuLDqkg@mail.gmail.com>
2018-02-27  0:07 ` Michael Eager [this message]
2018-02-27  8:14   ` Andrew Sadek
2018-02-26  7:56 Andrew Guirguis
2018-02-26  8:36 ` Andrew Sadek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ea62194-f081-0d2e-13f4-5884f1615e37@eagerm.com \
    --to=eager@eagerm.com \
    --cc=andrew.sadek.se@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox