Re: [Buildroot] [PATCH v4 1/3] board/andes: rearrange andes folder structure

[Yu Chien Peter Lin <peterlin@andestech.com>]

Hi Giulio,

On Thu, Feb 24, 2022 at 04:12:10PM +0100, Giulio Benetti wrote:
> Hi Peter,
> 
> On 24/02/22 03:41, Yu Chien Peter Lin wrote:
> > Hi Giulio,
> > 
> > > > > > > > Changes v3 -> v4:
> > > > > > > >       - rename ae3xx folder to ae300
> > > > > > > >       - rename andes_ae3xx_defconfig to andes_ae300_defconfig
> > > > > > > >       - update readme.txt and item in DEVELOPERS
> > > > > > > 
> > > > > > > Some of these change ^^^ must have been moved to the single patch, like
> > > > > > > DEVELOPERS is updated in patch 3/3. I've also forgotten to point you to add
> > > > > > > a --cover-letter, so keep in mind both points for next time.
> > > > > > > 
> > > > > > > I've built tested both defconfig successfully, so:
> > > > > > > Reviewed-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> > > > > > > Tested-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> > > > > > > [Giulio: build tested only]
> > > > > > > 
> > > > > > > And the other patches of this series is all reviewed, so to me it's ready to
> > > > > > > be committed, but this will happen after Buildroot 2022.02 is released,
> > > > > > > since between rc1 to the release Maintainers only accept fixes. Another
> > > > > > > possibility is that they will commit these patches to next branch.
> > > > > > > 
> > > > > > > Please remember to tag V3.
> > 
> > Oh, I thought you were refering to format-patch with v3, sorry.
> > I will mark the patch on patchwork to rejected state except the latest
> > patch version.
> 
> Ok, V3 it's still tagged as new in patchwork for the moment.

Updated to rejected state.

> > > > > > > Thank you for your effort!
> > > > > > > 
> > > > > > > Best regards
> > > > > > > -- 
> > > > > > > Giulio Benetti
> > > > > > > Benetti Engineering sas
> > > > > 
> > > > > Thanks for reviewing my patches and the guidance.
> > > > 
> > > > You're welcome.
> > > > 
> > > > > I will make a new patch for the Config.in,
> > > > 
> > > > Which one? I only see:
> > > > toolchain/toolchain-external/toolchain-external-andes-nds32/Config.in
> > > > 
> > > > Do you mean you will add an entry in DEVELOPERS for it?
> > 
> > I found the modifications on these files in Git history, usually have
> > its own patch.
> 
> No, maybe you mean there is a cover-letter that summarize the changes of all
> patches in patchset and then in every patch you list the changes of on that
> patch.
>
> > So in PATCH v5, I will split [PATCH 1/3] to [PATCH 1/4]
> > and [PATCH 2/4] for
> > toolchain/toolchain-external/toolchain-external-andes-nds32/Config.in
> > other patches remain the same,
> 
> No, wait. I've meant you only need to split:
> ---
> Changes v1->v2:
> *
> *
> [..]
> ---
> for every patch. So, don't summarize all the changes on patch [1/3], because
> those changes are related to different patches. For example in V1->V2 you
> state:
> - update DEVELOPERS
> but you do it in patch [3/3], so you only have to move the changes into the
> right patch, that's all.
> 
> > and attach [PATCH 0/4] cover letter.
> 
> And yes please, add a cover letter that briefly explain what patchset does
> and that's it.
> 
> > > > > The last question, is it tag V5 for the next submission?
> > > > 
> > > > It depends if you need to modify these patches. Or if you add a new patch
> > > > that depends on these patches, then yes, please send a V5 with all changes
> > > > moved to the different patches, including adding the patch if this is your
> > > > goal.
> > > > 
> > > > Best regards
> > > > -- 
> > > > Giulio Benetti
> > > > Benetti Engineering sas
> > 
> > Is there any the new comment?
> > Again, thanks for your help. :)
> 
> For the moment I don't see anything wrong except the changelog.
> 
> Thank you!
> 
> Best regards
> -- 
> Giulio Benetti
> Benetti Engineering sas

Thanks for clarification.

Best regards,
Yu Chien Peter Lin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot