Automated Standards Checking #3265
Conversation
dorodnic
force-pushed
the
standards_enforcement
branch
from
e99b554
to
62e4233
Compare
| fi | ||
|
|
||
| cd .. | ||
| check_folder include $1 |
There was a problem hiding this comment.
@dorodnic What about the wrappers folder? It contains files that are not updated (i.e python)
There was a problem hiding this comment.
At first, I did. In particular nodejs and python are obvious candidates.
However, both wrappers contain 3rd-party code within them that needs to be ignored.
While we should extend to wrappers as well, I'm ok to have only the core library covered for now.
| if [[ $(git check-ignore $filename | wc -l) -eq 0 ]]; then | ||
| if [[ $(grep -oP "(?<=\(c\) )(.*)(?= Intel)" $filename | wc -l) -eq 0 ]]; then | ||
| echo "[ERROR] $filename is missing the copyright notice" | ||
| ok=$((ok+1)) |
There was a problem hiding this comment.
@dorodnic why not $ok = 1?
There was a problem hiding this comment.
I want to count the number of errors to print it in the end.
This helps the user, as he can see this number get lower as he works through the issues.
| ok=$((ok+1)) | ||
|
|
||
| if [[ $2 == *"fix"* ]]; then | ||
| if [[ $(date +%Y) == "2019" ]]; then |
There was a problem hiding this comment.
@dorodnic I am sure that we will forget to update when we get to 2020 :)
There was a problem hiding this comment.
This is mostly due to my lack of bash knowledge. I'm using ex -sc '1i|// Copyright(c) 2019 Intel Corporation to append copyright before the first line.
However, if you try to use variables in that line (ex -sc '1i|// Copyright(c) $(date +%Y) Intel Corporation) you will just see "// Copyright(c) $(date +%Y) Intel Corporation" appended to the file. I tried to figure this out for half-an-hour and eventually gave up.
If someone knows how to do this better, will be happy to change it.
| ./pr_check.sh | ||
| else | ||
| if [[ ${ok} -ne 0 ]]; then | ||
| echo Pull-Request check failed, please address ${ok} the errors reported above |
common/android_helpers.h
Outdated
| @@ -1,5 +1,5 @@ | |||
| // License: Apache 2.0. See LICENSE file in root directory. | |||
| // Copyright(c) 2017 Intel Corporation. All Rights Reserved. | |||
| // Copyright(c) 2019 Intel Corporation. All Rights Reserved. | |||
There was a problem hiding this comment.
According to this (link) the year marks when the work was first published.
It is not really a rule, more a convention to keep the original date. But re-tagging the files, imho will produce extra-burden both for PR submitters and reviewers.
| } |
There was a problem hiding this comment.
Can you also add a check that a source file will always end with an empty line (like this one) ?
src/libuvc/ctrl.cpp
Outdated
| @@ -66,7 +66,7 @@ int uvc_get_ctrl_len(uvc_device_handle_t *devh, uint8_t unit, uint8_t ctrl) { | |||
| devh->usb_devh, | |||
| REQ_TYPE_GET, UVC_GET_LEN, | |||
| ctrl << 8, | |||
| unit << 8 | devh->info->ctrl_if.bInterfaceNumber, // XXX saki | |||
| unit << 8 | devh->info->ctrl_if.bInterfaceNumber, // XXX saki | |||
There was a problem hiding this comment.
The libuvc should probably be excluded, at least temporally. So then if we get a fix from a 3rd party to libuvc library, it will be much easier to compare and merge.
Actually, if we're going to make fixes into libuvc the script will automatically require us to add Intel's copyright as this is under /src
| target_sources(${LRS_TARGET} | ||
| PRIVATE | ||
| "${CMAKE_CURRENT_LIST_DIR}/cuda-align.h" | ||
| "${CMAKE_CURRENT_LIST_DIR}/cuda-align.cu" | ||
| "${CMAKE_CURRENT_LIST_DIR}/cuda-align.cuh" | ||
| "${CMAKE_CURRENT_LIST_DIR}/cuda-pointcloud.h" | ||
| "${CMAKE_CURRENT_LIST_DIR}/cuda-pointcloud.h" |
dorodnic
force-pushed
the
standards_enforcement
branch
from
62e4233
to
a71a34c
Compare
Somewhat necessary evil - this will enforce during CI the following requirements on every future PR:
Also updating all affected files to allow this to pass CI.