From: David Malcolm <dmalcolm@redhat.com>
To: Philip Herron <philip.herron@embecosm.com>, gcc-patches@gcc.gnu.org
Cc: manu@gcc.gnu.org
Subject: Re: Rust frontend patches v1
Date: Wed, 10 Aug 2022 15:06:50 -0400 [thread overview]
Message-ID: <7a069325c7db98d626cb589d260a62af16232ace.camel@redhat.com> (raw)
In-Reply-To: <CAB2u+n1V3P_+Qx6gxVvnLrfL=sFU6f2x4RzG3kpjs6XHoNy-5g@mail.gmail.com>
On Wed, 2022-08-10 at 19:56 +0100, Philip Herron wrote:
> Hi everyone
>
> For my v2 of the patches, I've been spending a lot of time ensuring
> each patch is buildable. It would end up being simpler if it was
> possible if each patch did not have to be like this so I could split
> up the front-end in more patches. Does this make sense?
Yes. I've often split up patches into chunks when posting them for
review, and I see other people here do that.
It makes it easier for the reviewer also, since it's usually easier to
deal with e.g. 10 small patches than one enormous one (e.g. if many of
them are uncontroversial, having them split up makes it easier to focus
attention on just the controversial areas).
> In theory,
> when everything goes well, does this still mean that we can merge in
> one commit,
Split up the patches for review, but make a note in the cover letter
than they would all be merged into one when committing.
(especially if doing the split is taking up a lot of time; we don't
want to be mandating busy-work)
Dave
> or should it follow a series of buildable patches? I've
> received feedback that it might be possible to ignore making each
> patch an independent chunk and just focus on splitting it up as small
> as possible even if they don't build.
>
> I hope this makes sense.
>
> Thanks
>
> --Phil
>
> On Thu, 28 Jul 2022 at 10:39, Philip Herron <
> philip.herron@embecosm.com> wrote:
> >
> > Thanks, for confirming David. I think it was too big in the end. I
> > was
> > trying to figure out how to actually split that up but it seems
> > reasonable that I can split up the front-end patches into patches
> > for
> > each separate pass in the compiler seems like a reasonable approach
> > for now.
> >
> > --Phil
> >
> > On Wed, 27 Jul 2022 at 17:45, David Malcolm <dmalcolm@redhat.com>
> > wrote:
> > >
> > > On Wed, 2022-07-27 at 14:40 +0100, herron.philip--- via Gcc-
> > > patches
> > > wrote:
> > > > This is the initial version 1 patch set for the Rust front-end.
> > > > There
> > > > are more changes that need to be extracted out for all the
> > > > target
> > > > hooks we have implemented. The goal is to see if we are
> > > > implementing
> > > > the target hooks information for x86 and arm. We have more
> > > > patches
> > > > for the other targets I can add in here but they all follow the
> > > > pattern established here.
> > > >
> > > > Each patch is buildable on its own and rebased ontop of
> > > > 718cf8d0bd32689192200d2156722167fd21a647. As for ensuring we
> > > > keep
> > > > attribution for all the patches we have received in the front-
> > > > end
> > > > should we create a CONTRIBUTOR's file inside the front-end
> > > > folder?
> > > >
> > > > Note thanks to Thomas Schwinge and Mark Wielaard, we are
> > > > keeping a
> > > > branch up to date with our code on:
> > > >
> > > > https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/devel/rust/master
> > > > but this is not rebased ontop of gcc head.
> > > >
> > > > Let me know if I have sent these patches correctly or not, this
> > > > is a
> > > > learning experience with git send-email.
> > > >
> > > > [PATCH Rust front-end v1 1/4] Add skeleton Rust front-end
> > > > folder
> > > > [PATCH Rust front-end v1 2/4] Add Rust lang TargetHooks for
> > > > i386 and
> > > > [PATCH Rust front-end v1 3/4] Add Rust target hooks to ARM
> > > > [PATCH Rust front-end v1 4/4] Add Rust front-end and associated
> > >
> > > FWIW it looks like patch 4 of the kit didn't make it (I didn't
> > > get a
> > > copy and I don't see it in the archives).
> > >
> > > Maybe it exceeded a size limit? If so, maybe try splitting it up
> > > into
> > > more patches.
> > >
> > > Dave
> > >
>
next prev parent reply other threads:[~2022-08-10 19:06 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 13:40 herron.philip
2022-07-27 13:40 ` [PATCH Rust front-end v1 1/4] Add skeleton Rust front-end folder herron.philip
2022-07-27 13:40 ` [PATCH Rust front-end v1 2/4] Add Rust lang TargetHooks for i386 and x86_64 herron.philip
2022-07-28 9:57 ` Jakub Jelinek
2022-07-28 10:38 ` Thomas Schwinge
2022-07-28 10:51 ` Philip Herron
2022-07-28 11:08 ` Thomas Schwinge
2022-07-28 11:41 ` Philip Herron
2022-07-27 13:40 ` [PATCH Rust front-end v1 3/4] Add Rust target hooks to ARM herron.philip
2022-07-27 14:13 ` Richard Earnshaw
2022-07-27 16:45 ` Rust frontend patches v1 David Malcolm
2022-07-28 9:39 ` Philip Herron
2022-08-10 18:56 ` Philip Herron
2022-08-10 19:06 ` David Malcolm [this message]
2022-08-12 20:45 ` Mike Stump
2022-08-15 14:07 ` Manuel López-Ibáñez
2022-08-15 14:33 ` Martin Liška
2022-08-24 13:48 ` Martin Liška
2022-08-24 13:51 ` Philip Herron
2022-08-24 14:02 ` Martin Liška
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=7a069325c7db98d626cb589d260a62af16232ace.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=manu@gcc.gnu.org \
--cc=philip.herron@embecosm.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).