| Message ID | 20190121151229.16744-1-niklas.soderlund@ragnatech.se |
|---|---|
| State | Rejected |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 21/01/2019 15:12, Niklas Söderlund wrote: > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/media_device.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > index 0ee55060aa9b5d23..a346296bc38e8f3d 100644 > --- a/src/libcamera/media_device.cpp > +++ b/src/libcamera/media_device.cpp > @@ -281,7 +281,7 @@ int MediaDevice::populate() > > /** > * \fn MediaDevice::devnode() > - * \brief Retrieve the media device device node path > + * \brief Retrieve the media device devnode path I would have just deleted one of the duplicates, and put "Retrieve the media device node path" device devnode feels a bit repetitive, because the 'dev' is short for device ;) -- Regards Kieran > * \return The MediaDevice devnode path > */ > >
Hi Kieran, On 2019-01-21 15:33:54 +0000, Kieran Bingham wrote: > Hi Niklas, > > On 21/01/2019 15:12, Niklas Söderlund wrote: > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/media_device.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > > index 0ee55060aa9b5d23..a346296bc38e8f3d 100644 > > --- a/src/libcamera/media_device.cpp > > +++ b/src/libcamera/media_device.cpp > > @@ -281,7 +281,7 @@ int MediaDevice::populate() > > > > /** > > * \fn MediaDevice::devnode() > > - * \brief Retrieve the media device device node path > > + * \brief Retrieve the media device devnode path > > I would have just deleted one of the duplicates, and put > "Retrieve the media device node path" > > device devnode feels a bit repetitive, because the 'dev' is short for > device ;) That's what I did at first. Then I read the \return description which uses devnode. I'm fine either way, let me know what you think. > > -- > Regards > > Kieran > > > > > * \return The MediaDevice devnode path > > */ > > > > > > -- > Regards > -- > Kieran
Hi Niklas, On 21/01/2019 15:38, Niklas Söderlund wrote: > Hi Kieran, > > On 2019-01-21 15:33:54 +0000, Kieran Bingham wrote: >> Hi Niklas, >> >> On 21/01/2019 15:12, Niklas Söderlund wrote: >>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> >>> --- >>> src/libcamera/media_device.cpp | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp >>> index 0ee55060aa9b5d23..a346296bc38e8f3d 100644 >>> --- a/src/libcamera/media_device.cpp >>> +++ b/src/libcamera/media_device.cpp >>> @@ -281,7 +281,7 @@ int MediaDevice::populate() >>> >>> /** >>> * \fn MediaDevice::devnode() >>> - * \brief Retrieve the media device device node path >>> + * \brief Retrieve the media device devnode path >> >> I would have just deleted one of the duplicates, and put >> "Retrieve the media device node path" >> >> device devnode feels a bit repetitive, because the 'dev' is short for >> device ;) > > That's what I did at first. Then I read the \return description which > uses devnode. I'm fine either way, let me know what you think. git blame points to Laurent on this line - so I'll defer to him if you want a final say. My feel would be that below is defining "MediaDevice" and "devnode" as the two entities are named in the code. Here in the brief, I would use more 'conversational' explanations. >> >> >>> * \return The MediaDevice devnode path >>> */ >>> >>> >
Hello, On Mon, Jan 21, 2019 at 03:57:37PM +0000, Kieran Bingham wrote: > On 21/01/2019 15:38, Niklas Söderlund wrote: > > On 2019-01-21 15:33:54 +0000, Kieran Bingham wrote: > >> On 21/01/2019 15:12, Niklas Söderlund wrote: > >>> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > >>> --- > >>> src/libcamera/media_device.cpp | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp > >>> index 0ee55060aa9b5d23..a346296bc38e8f3d 100644 > >>> --- a/src/libcamera/media_device.cpp > >>> +++ b/src/libcamera/media_device.cpp > >>> @@ -281,7 +281,7 @@ int MediaDevice::populate() > >>> > >>> /** > >>> * \fn MediaDevice::devnode() > >>> - * \brief Retrieve the media device device node path > >>> + * \brief Retrieve the media device devnode path > >> > >> I would have just deleted one of the duplicates, and put > >> "Retrieve the media device node path" > >> > >> device devnode feels a bit repetitive, because the 'dev' is short for > >> device ;) > > > > That's what I did at first. Then I read the \return description which > > uses devnode. I'm fine either way, let me know what you think. > > git blame points to Laurent on this line - so I'll defer to him if you > want a final say. > > My feel would be that below is defining "MediaDevice" and "devnode" as > the two entities are named in the code. Here in the brief, I would use > more 'conversational' explanations. I have a slight preference for spelling terms fully, both in code and documentation, unless abbreviations are necessary. This means "device node", "deviceNode", "setDeviceNode", ... If the team prefers the abbreviated version, that should be "devnode" and "setDevnode" used consistently through the code and comments. The most important point is consistency. > >>> * \return The MediaDevice devnode path > >>> */
diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index 0ee55060aa9b5d23..a346296bc38e8f3d 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -281,7 +281,7 @@ int MediaDevice::populate() /** * \fn MediaDevice::devnode() - * \brief Retrieve the media device device node path + * \brief Retrieve the media device devnode path * \return The MediaDevice devnode path */
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/media_device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)