Hey Philippe, Thanks for the detailed comments! I have a couple of questions.
1. I'll modify the patch to just include a fallthrough comment instead of an attribute. How do I include the v4 version number in the patch? Do I erase the last commit on my branch or fork from the master and start the work again and label it as v4? 2. I am trying to find some issues of interest, starting through which I can go to bigger contributions. Do you have any suggestions on how I might do that? For now, I am trying to tackle the bite sized issues to find my way around the code base. I would like to move to substantial contributions. 3. I have a background in CS theory, but its been 2 years since I graduated from my Master's so I am a bit rusty on some stuff. How much CS theory (like compilers and OS) do I need to know if I want to contribute? Thanks, Rohit. On Sat, Aug 15, 2020 at 11:24 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > Hi Rohit, > > Congratulation for your first patch! It is in very > good shape already :) > > It is easier for the reviewers if you start the patch subject with > the name of the subsystem concerned, or the file modified: > > "qapi/opts-visitor: Add missing fallthrough annotations" > > On 8/15/20 3:00 PM, Rohit Shinde wrote: > > Added the fallthrough comment so that the compiler doesn't emit an error > on compiling with the -Wimplicit-fallthrough flag. > > If possible align the description to 72 chars. > > > > > Signed off by: Rohit Shinde > > The tag is written "Signed-off-by" with '-', then your "name <email>": > > Signed-off-by: Rohit Shinde <rohit.shinde12...@gmail.com> > > If you configure your git client, using 'git-commit -s' will > automatically add the S-o-b tag: > > $ git config user.name "Rohit Shinde" > $ git config user.email "rohit.shinde12...@gmail.com" > $ git commit -s > > > --- > > qapi/opts-visitor.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > > index 7781c23a42..43cf60d3a0 100644 > > --- a/qapi/opts-visitor.c > > +++ b/qapi/opts-visitor.c > > @@ -266,6 +266,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t > size) > > } > > ov->list_mode = LM_IN_PROGRESS; > > /* range has been completed, fall through in order to pop > option */ > > + __attribute__((fallthrough)); > > C uses attributes when declaring a type/variable/function. > Here this is inside a function body, not a declaration. > A simple "/* fallthrough */" comment will make the compiler happy. > You can see a similar patch for example: > > https://git.qemu.org/?p=qemu.git;a=blobdiff;f=disas/sh4.c;h=dcdbdf26d8;hp=55ef865a3;hb=ccb237090f;hpb=7aa12aa215 > > When you find an issue that might have already been fixed elsewhere > in the repository, 'git-log -p' is your friend. Since the commits are > patches already accepted/merged, they might be a good source to learn > (how the issue was fixed, how the bug was described, ...). > Regards, > > Phil. > > > > > case LM_IN_PROGRESS: { > > const QemuOpt *opt; > > > >
