Resolve "Add clang format to the CI tests"
Closes #13 (closed)
Merge request reports
Activity
added 1 commit
- dd422494 - Temporarily run clang-format in precheck stage for faster iteration
added 1 commit
- 3d3e4d66 - Revert "Only clone clang-format config file in relevant job"
- Resolved by Prada Sarasola, Miguel
I think the
clang-format
check is ready.I decided to add the
wget
command fetching the formatting configuration in the.industrial_ci
template, which isn't ideal. Adding it in the specific job requires repeating the otherbefore_script
steps, because they are otherwise overwritten.I also left the
except: melodic-devel
flag because in my own repos I found out that the default version ofclang-format
which is installed inxenial
andbionic
have slightly different behaviors and jobs that pass in one distro may fail in the other.Anyways, a couple of jobs showing the job in action: one where the check succeeds and other where the check fails.
After those jobs I pushed two new commits:
- moving the job to the
postcheck
stage (it was faster to iterate having it in theprecheck
stage) -
wget
-ing theclang-format
config file frommaster
, in praparation to merge
What do you think?
- moving the job to the
assigned to @jon.azpiazu
- Resolved by Prada Sarasola, Miguel
- Resolved by Prada Sarasola, Miguel
- Resolved by Prada Sarasola, Miguel
- Resolved by Prada Sarasola, Miguel
added 5 commits
- f768ff9d - Explicitly add wget in case it's removed from base image
- 161cb6bc - Add comment to clang-format file pointing to its source
- 17e9951c - Run clang-format job for tags as well
- 51687632 - Force specific version of clang-format (3.8, default in Xenial)
- 7692a7a0 - Enable clang-format job for melodic-devel branch
Toggle commit list- Resolved by Jon Azpiazu
Here's a test job after the latest changes.
Some minor suggestions:
- do not
wget
in the.industrial_ci
job, as it forces to download for every CI job when it is not really needed - in the same spirit, do not extend from the
.industrial_ci
job: we are forcing to login to artifactory even if it is not needed. I do not see a clear benefit of extending for this case. - I am not sure if it is possible, but use a smaller image - it is currently using
ubuntu:xenial
when probablyalpine
is enough.
- do not
added 1 commit
- dc90b463 - Make clang_format job not to extend .industrial_ci
Addressed two of your last comments in dc90b463.
W/respect of using a smaller base image,
alpine
will not work because the clang-format check usesapt-get
to download its dependencies. Maybe there's some smaller ubuntu or debian-based image that may work, but I'm honestly not very excited to spend time on this.