Em sex, 2021-04-23 às 11:21 +0200, Hans Verkuil escreveu: > On 21/04/2021 16:21, Aline Santana Cordeiro wrote: > > Em qua, 2021-04-21 às 15:56 +0200, Julia Lawall escreveu: > > > > > > > > > On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote: > > > > > > > Em qua, 2021-04-21 às 15:08 +0200, Julia Lawall escreveu: > > > > > > > > > > > > > > > On Wed, 21 Apr 2021, Aline Santana Cordeiro wrote: > > > > > > > > > > > Change line break to avoid an open parenthesis at the end > > > > > > of > > > > > > the > > > > > > line. > > > > > > It consequently removed spaces at the start of the > > > > > > subsequent > > > > > > line. > > > > > > > > > > The message is hard to understand. There are a lot of > > > > > singular > > > > > nouns, but > > > > > actually there are two changes. Which change is being > > > > > described > > > > > by > > > > > the > > > > > above message? What does "It" refer to? > > > > > > > > > > julia > > > > > > > > Checkpatch indicated two problems with this function > > > > declaration: > > > > 1) The line ending with an open parenthesis, and > > > > 2) The following line - with the function parameters - has > > > > spaces > > > > in > > > > its identation. > > > > > > > > When I changed the line break to put the function name and its > > > > parameter in the following line, both checkpath checks were > > > > eliminated. > > > > > > > > So, the main change was the line break and, also, the line > > > > break > > > > (it) > > > > removed the space in the following line. > > > > > > > > Is it better to change the message and explain only about the > > > > line > > > > break? > > > > > > The message should explain about the whole patch. So if you > > > change > > > two > > > things, it should be clear that what you are saying covers both > > > of > > > them. > > > > Ok, I can do that. In the commit message I described just one issue > > because it is only one patch, I didn't want it to look like I was > > changing different issues in just one patch. > > > > > > > > But it seems that Matthew doesn't think that the line break is a > > > good > > > idea > > > anyway. > > > > Yes, I'm sending this email to Matthew too, because I don't know > > exactly how to proceed as Hans asked me to made some corrections > > too. > > I've made these changes because checkpatch has indicated and with > > this > > line break, checkpatch does not indicate any check or warning > > anymore. > > But I can undo that too, I just don't know what I'm supposed to do > > with > > so many opposite opinions. > > As one of the media maintainers I can say that in this case the > preference > would be to split it up in two lines. It's one of those areas where > different maintainers have different opinions. > > Just keep in mind that this is all nitpicking and normally we > probably > wouldn't bother with this at all, but it is a good exercise to learn > about patches and contributing :-) > > Regards, > > Hans I really appreciate all the feedbacks I've received :) Indeed we can learn a lot about all the contributing process. Thank you, Aline > > > > > > > Thank you all, > > Aline > > > > > > julia > > > > > > > > > > > Thank you, > > > > Aline > > > > > > > > > > > > > > > > Both issues detected by checkpatch.pl. > > > > > > > > > > > > Signed-off-by: Aline Santana Cordeiro < > > > > > > alinesantanacordeiro@xxxxxxxxx> > > > > > > --- > > > > > > > > > > > > Changes since v2: > > > > > > - Insert a space between the function type and pointer > > > > > > > > > > > > Changes since v1: > > > > > > - Keep the pointer with the function return type > > > > > > instead of left it with the function name > > > > > > > > > > > > drivers/staging/media/atomisp/pci/atomisp_cmd.h | 10 > > > > > > +++++---- > > > > > > - > > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > > > diff --git > > > > > > a/drivers/staging/media/atomisp/pci/atomisp_cmd.h > > > > > > b/drivers/staging/media/atomisp/pci/atomisp_cmd.h > > > > > > index 1c0d464..639eca3 100644 > > > > > > --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h > > > > > > +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h > > > > > > @@ -75,8 +75,8 @@ void atomisp_wdt(struct timer_list *t); > > > > > > void atomisp_setup_flash(struct atomisp_sub_device *asd); > > > > > > irqreturn_t atomisp_isr(int irq, void *dev); > > > > > > irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr); > > > > > > -const struct atomisp_format_bridge > > > > > > *get_atomisp_format_bridge_from_mbus( > > > > > > - u32 mbus_code); > > > > > > +const struct atomisp_format_bridge * > > > > > > +get_atomisp_format_bridge_from_mbus(u32 mbus_code); > > > > > > bool atomisp_is_mbuscode_raw(uint32_t code); > > > > > > int atomisp_get_frame_pgnr(struct atomisp_device *isp, > > > > > > const struct ia_css_frame > > > > > > *frame, > > > > > > u32 > > > > > > *p_pgnr); > > > > > > @@ -381,9 +381,9 @@ enum mipi_port_id > > > > > > __get_mipi_port(struct > > > > > > atomisp_device *isp, > > > > > > > > > > > > bool atomisp_is_vf_pipe(struct atomisp_video_pipe *pipe); > > > > > > > > > > > > -void atomisp_apply_css_parameters( > > > > > > - struct atomisp_sub_device *asd, > > > > > > - struct atomisp_css_params *css_param); > > > > > > +void atomisp_apply_css_parameters(struct > > > > > > atomisp_sub_device > > > > > > *asd, > > > > > > + struct atomisp_css_params > > > > > > *css_param); > > > > > > + > > > > > > void atomisp_free_css_parameters(struct atomisp_css_params > > > > > > *css_param); > > > > > > > > > > > > void atomisp_handle_parameter_and_buffer(struct > > > > > > atomisp_video_pipe > > > > > > *pipe); > > > > > > -- > > > > > > 2.7.4 > > > > > > > > > > > > -- > > > > > > You received this message because you are subscribed to the > > > > > > Google > > > > > > Groups "outreachy-kernel" group. > > > > > > To unsubscribe from this group and stop receiving emails > > > > > > from > > > > > > it, > > > > > > send an email to > > > > > > outreachy-kernel+unsubscribe@xxxxxxxxxxxxxxxx. > > > > > > To view this discussion on the web visit > > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20210421123718.GA4597%40focaruja > > > > > > . > > > > > > > > > > > > > > > > > > -- > > > > You received this message because you are subscribed to the > > > > Google > > > > Groups "outreachy-kernel" group. > > > > To unsubscribe from this group and stop receiving emails from > > > > it, > > > > send an email to outreachy-kernel+unsubscribe@xxxxxxxxxxxxxxxx. > > > > To view this discussion on the web visit > > > > https://groups.google.com/d/msgid/outreachy-kernel/7aeac7041a6f6d7b3d8563f0d0bf0a4d31f379b0.camel%40gmail.com > > > > . > > > > >
