[libcamera-devel] [PATCH 4/4] ipa: raspberrypi: Tidy up variable names to be consistent
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Sep 23 13:45:33 CEST 2020
Hi Jacopo,
On 23/09/2020 12:39, Jacopo Mondi wrote:
> /me dives into bikeshedding
<snip>
>>> @@ -134,11 +134,11 @@ private:
>>> * We count frames to decide if the frame must be hidden (e.g. from
>>> * display) or mistrusted (i.e. not given to the control algos).
>>> */
>>> - uint64_t frame_count_;
>>> + uint64_t frameCount_;
>>
>> If we're doing clean up - I always prefer to see a newline before a
>> comment line.
Note here I state 'before a comment'
>> To me it makes the commented block clearer - but I don't know if there's
>> a 'rule' on it.
>>
>
> I'm sorry but
>
> /*
> * A comment describing a variable is better places as close
> * as possible near to that variable, isn't it ?
> */
> int whatAFancyVariable;
>
> /* Or do you think this variable is not fancy and an empty line is better? */
>
Here you seem to be implying 'after' a comment...
> int notAFancyVariable;
>
> Ok, enough useless discussions from me :)
>
I ... 'think' you mis-interpreted what I said, or I was not clear. I was
probably not clear ;-)
I think that this:
/* A variable group. */
bool variable;
int quantity;
/*
* We count frames to decide if the frame must be hidden (e.g.
* from display) or mistrusted (i.e. not given to the control
* algos).
*/
uint64_t frameCount_;
/* For checking the sequencing of Prepare/Process calls. */
uint64_t checkCount_;
/* How many frames we should avoid running control algos on. */
unsigned int mistrustCount_;
is better than this:
/* A variable group */
bool variable;
int quanty;
/*
* We count frames to decide if the frame must be hidden (e.g.
* from display) or mistrusted (i.e. not given to the control
* algos).
*/
uint64_t frameCount_;
/* For checking the sequencing of Prepare/Process calls. */
uint64_t checkCount_;
/* How many frames we should avoid running control algos on. */
unsigned int mistrustCount_;
Maybe one day I'll finish my comment formatter in the checkstyle script ;-)
>>
>>> /* For checking the sequencing of Prepare/Process calls. */
>>> - uint64_t check_count_;
>>> + uint64_t checkCount_;
>>
>> i.e. here too.
>>
>>> /* How many frames we should avoid running control algos on. */
>>> - unsigned int mistrust_count_;
>>> + unsigned int mistrustCount_;
>>
>> and so on.
>>
>>> /* LS table allocation passed in from the pipeline handler. */
>>> FileDescriptor lsTableHandle_;
>>> void *lsTable_;
<snip>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list