| Message ID | 20200623105730.26956-1-chris@gregariousmammal.com |
|---|---|
| State | Accepted |
| Commit | e284261cf066fa9911ce75da6a746b1379a4a2a8 |
| Headers | show |
| Series |
|
| Related | show |
Hi Chris, Did you re-send this patch by mistake? It's identical with the patch you sent on 6/15/2020. When you re-send the patch, you need to also mention that the patch is the second (or third, fourth ...) iteration of the initial patch. You can do that by adding "-v2" to "git format-patch ..." commandline to denote that it's the second version of that patch. Also, since you got Reviewed-by tags by me and Laurent, you need to collect them too and add it to your commit message while re-sending them for the second round of review. On 6/23/20 4:27 PM, chris@gregariousmammal.com wrote: > From: Chris Chinchilla <chris@gregariousmammal.com> > > Fix incorrect method in cameradata docs > > Fixes: b581b9576abd ("libcamera: pipeline_handler: Make pipeline-specific data mandatory") > Signed-off-by: Chris Chinchilla <chris@gregariousmammal.com> > --- > src/libcamera/pipeline_handler.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 14dfba0..15cdc17 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) > * > * Pipeline handlers are expected to extend this base class with platform > * specific implementation, associate instances of the derived classes > - * using the setCameraData() method, and access them at a later time > + * using the registerCamera() method, and access them at a later time > * with cameraData(). > */ >
Yes, I resent it as people told me it was missing tags :) So it’s missing more tags now? I admit, I didn’t see the reviewed by tags appear anywhere. So to clarify, to get this right, I need to resend again with a v3 and add reviewed by tags? Chris On 23 Jun 2020, 19:38 +0200, Umang Jain <email@uajain.com>, wrote: > Hi Chris, > > Did you re-send this patch by mistake? It's identical with the patch > you sent on 6/15/2020. > > When you re-send the patch, you need to also mention that the patch is > the second (or third, fourth ...) iteration of the initial patch. > You can do that by adding "-v2" to "git format-patch ..." commandline > to denote that it's the second version of that patch. > > Also, since you got Reviewed-by tags by me and Laurent, you need to collect > them too and add it to your commit message while re-sending them for the > second round of review. > > On 6/23/20 4:27 PM, chris@gregariousmammal.com wrote: > > From: Chris Chinchilla <chris@gregariousmammal.com> > > > > Fix incorrect method in cameradata docs > > > > Fixes: b581b9576abd ("libcamera: pipeline_handler: Make pipeline-specific data mandatory") > > Signed-off-by: Chris Chinchilla <chris@gregariousmammal.com> > > --- > > src/libcamera/pipeline_handler.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > > index 14dfba0..15cdc17 100644 > > --- a/src/libcamera/pipeline_handler.cpp > > +++ b/src/libcamera/pipeline_handler.cpp > > @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) > > * > > * Pipeline handlers are expected to extend this base class with platform > > * specific implementation, associate instances of the derived classes > > - * using the setCameraData() method, and access them at a later time > > + * using the registerCamera() method, and access them at a later time > > * with cameraData(). > > */ > >
Hi Chris, On 25/06/2020 09:33, Chris Chinchilla wrote: > Yes, I resent it as people told me it was missing tags :) > > So it’s missing more tags now? I admit, I didn’t see the reviewed by > tags appear anywhere. > > So to clarify, to get this right, I need to resend again with a v3 and > add reviewed by tags? No need to resend, I can add the tags when applying. The Reviewed-by: tags were given to you via e-mail in reply to your previous post. When posting a revised version of a patch, usually the patch author would collect previously given tags, because tags mean integration. Also when posting a second version of a patch, you can mark it with an increased version number, so this would be: [PATCH v2] Fix incorrect method in cameradata docs When you save the patch with git, you can specify the version to do that automatically, just add a '-v2' (or '-'v3'...) to the command git format-patch -1 -v2 The file names of the patches generated will be prefixed with the version number. But, this patch has already got two Reviewed-by: tags, and the commit message has been fixed up according to the previous comments. So I'll add the Reviewed-by: (RB) tags and integrate (unless someone already has...) -- Thanks Kieran > > Chris > On 23 Jun 2020, 19:38 +0200, Umang Jain <email@uajain.com>, wrote: >> Hi Chris, >> >> Did you re-send this patch by mistake? It's identical with the patch >> you sent on 6/15/2020. >> >> When you re-send the patch, you need to also mention that the patch is >> the second (or third, fourth ...) iteration of the initial patch. >> You can do that by adding "-v2" to "git format-patch ..." commandline >> to denote that it's the second version of that patch. >> >> Also, since you got Reviewed-by tags by me and Laurent, you need to >> collect >> them too and add it to your commit message while re-sending them for the >> second round of review. >> >> On 6/23/20 4:27 PM, chris@gregariousmammal.com wrote: >>> From: Chris Chinchilla <chris@gregariousmammal.com> >>> >>> Fix incorrect method in cameradata docs >>> >>> Fixes: b581b9576abd ("libcamera: pipeline_handler: Make >>> pipeline-specific data mandatory") >>> Signed-off-by: Chris Chinchilla <chris@gregariousmammal.com> >>> --- >>> src/libcamera/pipeline_handler.cpp | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/libcamera/pipeline_handler.cpp >>> b/src/libcamera/pipeline_handler.cpp >>> index 14dfba0..15cdc17 100644 >>> --- a/src/libcamera/pipeline_handler.cpp >>> +++ b/src/libcamera/pipeline_handler.cpp >>> @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) >>> * >>> * Pipeline handlers are expected to extend this base class with platform >>> * specific implementation, associate instances of the derived classes >>> - * using the setCameraData() method, and access them at a later time >>> + * using the registerCamera() method, and access them at a later time >>> * with cameraData(). >>> */ >>> > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
Hi Chris, On 6/25/20 2:03 PM, Chris Chinchilla wrote: > Yes, I resent it as people told me it was missing tags :) > > So it’s missing more tags now? I admit, I didn’t see the reviewed by > tags appear anywhere. Did you see commit history in libcamera git repo? $ cd /path/to/libcamera/repo $ git log See couple of commits and their messages: They will have: * Signed-off-by tag - This generally belongs to the author of the commit * Reviewed-by tag - This is to denote who reviewed your commit. Obviously, v1 won't have any Reviewed-by tags. So, when from submit next version (v2, v3...) You need to append the commit message, if you have got "Reviewed-by" tag by the reviewer when they reviewed our patches on the mailing list. You need 2-or-more "Reviewed-by tags" to get your patches merged. This is not a hard rule but that's what I have been told :) > > So to clarify, to get this right, I need to resend again with a v3 and > add reviewed by tags? Yes, apart from the fixes pointed out in the last review (if any), you need to send a next version (v3), making sure it has signoff and reviewed-by tags included from the review. This is generic workflow. Since, I checked, this particular patch acutally fixes an old commit, you need to specify: "Fixes: " tag as mentioned in the previous review. So in total, you will have 1 "Fixes: " tag, 1 Signoff tag and 2 Reviewed-by tags (mine and Laurent's). Hope I have made this clear. Please feel free to reach out if you have any more questions. > > Chris > On 23 Jun 2020, 19:38 +0200, Umang Jain <email@uajain.com>, wrote: >> Hi Chris, >> >> Did you re-send this patch by mistake? It's identical with the patch >> you sent on 6/15/2020. >> >> When you re-send the patch, you need to also mention that the patch is >> the second (or third, fourth ...) iteration of the initial patch. >> You can do that by adding "-v2" to "git format-patch ..." commandline >> to denote that it's the second version of that patch. >> >> Also, since you got Reviewed-by tags by me and Laurent, you need to >> collect >> them too and add it to your commit message while re-sending them for the >> second round of review. >> >> On 6/23/20 4:27 PM, chris@gregariousmammal.com wrote: >>> From: Chris Chinchilla <chris@gregariousmammal.com> >>> >>> Fix incorrect method in cameradata docs >>> >>> Fixes: b581b9576abd ("libcamera: pipeline_handler: Make >>> pipeline-specific data mandatory") >>> Signed-off-by: Chris Chinchilla <chris@gregariousmammal.com> >>> --- >>> src/libcamera/pipeline_handler.cpp | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/libcamera/pipeline_handler.cpp >>> b/src/libcamera/pipeline_handler.cpp >>> index 14dfba0..15cdc17 100644 >>> --- a/src/libcamera/pipeline_handler.cpp >>> +++ b/src/libcamera/pipeline_handler.cpp >>> @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) >>> * >>> * Pipeline handlers are expected to extend this base class with platform >>> * specific implementation, associate instances of the derived classes >>> - * using the setCameraData() method, and access them at a later time >>> + * using the registerCamera() method, and access them at a later time >>> * with cameraData(). >>> */ >>>
Hi Chris, On 23/06/2020 11:57, chris@gregariousmammal.com wrote: > From: Chris Chinchilla <chris@gregariousmammal.com> > > Fix incorrect method in cameradata docs We normally wouldn't just repeat the $SUBJECT. I'll replace this with: """ The pipeline handler documentation incorrectly references an old API usage of setCameraData, which should have been updated to registerCamera() while updating pipeline handlers to ensure they all have a pipeline-specific "CameraData" allocation. Update the remaining documentation reference. """ > > Fixes: b581b9576abd ("libcamera: pipeline_handler: Make pipeline-specific data mandatory") > Signed-off-by: Chris Chinchilla <chris@gregariousmammal.com> And I'll add the following: Reviewed-by: Umang Jain <email@uajain.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks. > --- > src/libcamera/pipeline_handler.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 14dfba0..15cdc17 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) > * > * Pipeline handlers are expected to extend this base class with platform > * specific implementation, associate instances of the derived classes > - * using the setCameraData() method, and access them at a later time > + * using the registerCamera() method, and access them at a later time > * with cameraData(). > */ > >
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 14dfba0..15cdc17 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -46,7 +46,7 @@ LOG_DEFINE_CATEGORY(Pipeline) * * Pipeline handlers are expected to extend this base class with platform * specific implementation, associate instances of the derived classes - * using the setCameraData() method, and access them at a later time + * using the registerCamera() method, and access them at a later time * with cameraData(). */