[libcamera-devel] Fix incorrect method in cameradata docs

Message ID 20200615144919.42388-1-chris@gregariousmammal.com
State Superseded
Headers show
Series
  • [libcamera-devel] Fix incorrect method in cameradata docs
Related show

Commit Message

Chris Chinchilla June 15, 2020, 2:49 p.m. UTC
From: Chris Chinchilla <chris@gregariousmammal.com>

---
 src/libcamera/pipeline_handler.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Umang Jain June 15, 2020, 3:14 p.m. UTC | #1
Hi Chris,

On 6/15/20 8:19 PM, chris@gregariousmammal.com wrote:
> From: 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 a0f6b0f..fca11cb 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().
>    */
>   

The change looks good. :-)

Couple of comments on the commit message.

* Missing sign-off tag

    - Patches need to signed off. You can use --signoff flag while 
committing (git commit --signoff ...)

* Missing "Fixes:" tag
    - This commit actually fixes commit: b581b9576abd ("libcamera: 
pipeline_handler: Make pipeline-specific data mandatory"), so add this 
tag to the commit message:

 > Fixes: b581b9576abd ("libcamera: pipeline_handler: Make 
pipeline-specific data mandatory")


Once you have these two things in your commit message:

Reviewed-by: Umang Jain <email@uajain.com>
Laurent Pinchart June 16, 2020, 2:49 a.m. UTC | #2
On Mon, Jun 15, 2020 at 03:14:26PM +0000, Umang Jain wrote:
> On 6/15/20 8:19 PM, chris@gregariousmammal.com wrote:

We try to always include a commit message, even for simple changes. It's
not a hard requirements, but it's highly encouraged.

> > From: 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 a0f6b0f..fca11cb 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().
> >    */
> 
> The change looks good. :-)
> 
> Couple of comments on the commit message.
> 
> * Missing sign-off tag
> 
>     - Patches need to signed off. You can use --signoff flag while 
> committing (git commit --signoff ...)

Or -s, that's shorter. And we even document the Signed-off-by
requirement in http://libcamera.org/contributing.html#submitting-patches
:-)

> 
> * Missing "Fixes:" tag
>     - This commit actually fixes commit: b581b9576abd ("libcamera: 
> pipeline_handler: Make pipeline-specific data mandatory"), so add this 
> tag to the commit message:
> 
> Fixes: b581b9576abd ("libcamera: pipeline_handler: Make pipeline-specific data mandatory")

To add a bit more information, 'git blame' can help checking which
commit last modified a given line. To generate the Fixes: line, I have
added the following in ~/.gitconfig:

[pretty]
	fixes = Fixes: %h (\"%s\")

and added the following function in my ~.bashrc:

gpf() {
	git show --pretty=fixes ${1:-HEAD} | head -1
}

Running

gpf b581b9576abd

produces

Fixes: b581b9576abd ("libcamera: pipeline_handler: Make pipeline-specific data mandatory")

> Once you have these two things in your commit message:
> 
> Reviewed-by: Umang Jain <email@uajain.com>

Same,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Patch

diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index a0f6b0f..fca11cb 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().
  */