[libcamera-devel] [RFC PATCH] libcamera: Rename 'method' to 'function'

[Kieran Bingham]

On 06/08/2021 11:10, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Fri, Aug 06, 2021 at 10:57:50AM +0100, Kieran Bingham wrote:
>> On 05/08/2021 17:35, Laurent Pinchart wrote:
>>> Usage of 'method' to refer to member functions comes from Java. The C++
>>> standard uses the term 'function' only. Replace 'method' with 'function'
>>> or 'member function' through the whole code base and documentation.
>>
>> Ouch ;-)
>>
>> I think method is widely known as a 'function of a class' ... but indeed
>> that's also more correctly known as a member function I think.
>>
>> So - using 'function' over 'method' is perhaps more correct for C++.
>>
>>
>> I won't nitpick over this for specific reviews, as long as it doesn't
>> break compilation, and checkstyle is happy, I'm sure you've dealt with
>> line lengths and reflows accordingly.
> 
> Paul went through the patch and found a typo in a comment :-)

Perfect, see - I don't need to hehe

> 
>> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>> I do wonder if you might consider adding the rule to checkstyle though ...
> 
> Yes, but I think we should rename BoundMethod first in that case.
> Otherwise there will be too many false positives.
> 
> Work on BoundMethod is on my todo list, I'd like to investigate if the
> custom implementation could be replaced (or at least simplified) with
> usage of C++17 features such as std::apply, but that's not high
> priority.

Is BoundMethod exposed as public libcamera API - which would currently
prevent C++17 ..?


> 
>> But - perhaps that might just be a generic 's/method/function/' on the
>> whole patch content to see if it picks up any changes?
>>
>> Essentially it's a custom spell check correction ... hrm ...
>>
>>> The BoundMethod and Object::invokeMethod() are left as-is here, and will
>>> be addressed separately.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>> I've carried this in my tree for too long, and it's very annoying to
>>> rebase as you can imagine, so I thought I'd sent it out to get it
>>> accepted or rejected, but out of its current limbo.
>>> ---
>>>  Documentation/coding-style.rst                |   2 +-
>>>  Documentation/docs.rst                        |   2 +-
>>>  .../guides/application-developer.rst          |   4 +-
>>>  Documentation/guides/ipa.rst                  |  10 +-
>>>  Documentation/guides/pipeline-handler.rst     | 102 +++++++++---------
>>>  include/libcamera/ipa/core.mojom              |   2 +-
>>>  include/libcamera/ipa/raspberrypi.mojom       |  31 +++---
>>>  src/android/camera_device.cpp                 |   2 +-
>>>  src/android/jpeg/exif.cpp                     |   2 +-
>>>  src/ipa/ipu3/ipu3.cpp                         |   2 +-
>>>  src/ipa/libipa/camera_sensor_helper.cpp       |   2 +-
>>>  src/ipa/raspberrypi/cam_helper.cpp            |   4 +-
>>>  src/ipa/raspberrypi/cam_helper.hpp            |  14 +--
>>>  src/ipa/raspberrypi/controller/controller.hpp |   4 +-
>>>  src/ipa/raspberrypi/controller/rpi/alsc.hpp   |   2 +-
>>>  .../raspberrypi/controller/rpi/sharpen.cpp    |   2 +-
>>>  src/ipa/raspberrypi/md_parser.hpp             |   4 +-
>>>  src/ipa/raspberrypi/raspberrypi.cpp           |   2 +-
>>>  src/libcamera/base/event_dispatcher_poll.cpp  |   2 +-
>>>  src/libcamera/base/log.cpp                    |  10 +-
>>>  src/libcamera/base/message.cpp                |   6 +-
>>>  src/libcamera/base/object.cpp                 |  22 ++--
>>>  src/libcamera/base/semaphore.cpp              |  16 +--
>>>  src/libcamera/base/signal.cpp                 |   2 +-
>>>  src/libcamera/base/thread.cpp                 |  40 +++----
>>>  src/libcamera/byte_stream_buffer.cpp          |   9 +-
>>>  src/libcamera/camera.cpp                      |  24 ++---
>>>  src/libcamera/camera_manager.cpp              |   2 +-
>>>  src/libcamera/camera_sensor.cpp               |  24 ++---
>>>  src/libcamera/control_serializer.cpp          |   6 +-
>>>  src/libcamera/control_validator.cpp           |   2 +-
>>>  src/libcamera/controls.cpp                    |  20 ++--
>>>  src/libcamera/device_enumerator.cpp           |   2 +-
>>>  src/libcamera/file_descriptor.cpp             |  14 +--
>>>  src/libcamera/formats.cpp                     |   2 +-
>>>  src/libcamera/framebuffer.cpp                 |  10 +-
>>>  src/libcamera/framebuffer_allocator.cpp       |   4 +-
>>>  src/libcamera/geometry.cpp                    |   2 +-
>>>  src/libcamera/ipa_interface.cpp               |  14 +--
>>>  src/libcamera/ipa_manager.cpp                 |  11 +-
>>>  src/libcamera/ipa_module.cpp                  |  12 +--
>>>  src/libcamera/ipa_proxy.cpp                   |   4 +-
>>>  src/libcamera/ipc_unixsocket.cpp              |  22 ++--
>>>  src/libcamera/media_device.cpp                |   4 +-
>>>  src/libcamera/pipeline/ipu3/imgu.cpp          |   4 +-
>>>  src/libcamera/pipeline_handler.cpp            |  58 +++++-----
>>>  src/libcamera/process.cpp                     |  10 +-
>>>  src/libcamera/request.cpp                     |  11 +-
>>>  src/libcamera/stream.cpp                      |  17 +--
>>>  src/libcamera/v4l2_device.cpp                 |  20 ++--
>>>  src/libcamera/v4l2_videodevice.cpp            |  16 +--
>>>  test/media_device/media_device_link_test.cpp  |   2 +-
>>>  test/signal.cpp                               |   4 +-
>>>  53 files changed, 313 insertions(+), 308 deletions(-)
> 
> [snip]
>