-
Notifications
You must be signed in to change notification settings - Fork 101
adding clang-tidy as a linter tool #2269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
1d93369
f724d88
efeb4fa
31aead3
600c035
9935304
5e54b91
d64fdaa
498e2a3
84e8418
df5dc10
7ba7cab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| sudo apt-get install -y clang-tidy | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,61 @@ | ||||||||||||||||||||||||||
| - job: | ||||||||||||||||||||||||||
| name: ceph-pr-clang-tidy | ||||||||||||||||||||||||||
| node: small | ||||||||||||||||||||||||||
| project-type: freestyle | ||||||||||||||||||||||||||
| defaults: global | ||||||||||||||||||||||||||
| display-name: 'ceph: Clang-tidy checks' | ||||||||||||||||||||||||||
| concurrent: true | ||||||||||||||||||||||||||
| quiet-period: 5 | ||||||||||||||||||||||||||
| block-downstream: false | ||||||||||||||||||||||||||
| block-upstream: false | ||||||||||||||||||||||||||
| retry-count: 3 | ||||||||||||||||||||||||||
| properties: | ||||||||||||||||||||||||||
| - build-discarder: | ||||||||||||||||||||||||||
| days-to-keep: 15 | ||||||||||||||||||||||||||
| artifact-days-to-keep: 15 | ||||||||||||||||||||||||||
| - github: | ||||||||||||||||||||||||||
| url: https://github.com/ceph/ceph/ | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| parameters: | ||||||||||||||||||||||||||
| - string: | ||||||||||||||||||||||||||
| name: ghprbPullId | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| parameters: | |
| - string: | |
| name: ghprbPullId | |
| description: "the GitHub pull id, like '72' in 'ceph/pull/72'" |
ceph-build/ceph-perf-pull-requests/config/definitions/ceph-perf-pull-requests.yml
Lines 137 to 140 in e8d18c7
| parameters: | |
| - string: | |
| name: ghprbPullId | |
| description: "the GitHub pull id, like '72' in 'ceph/pull/72'" |
| parameters: | |
| - string: | |
| name: ghprbPullId | |
| description: "the GitHub pull id, like '72' in 'ceph/pull/72'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmick and what about this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can leave it in for now; I don't think it's necessary but it might be useful for testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of course this only works if the build machine is an apt-based system, which it may not be using your current node labels. It might be more appropriate to install the package inside the pbuilder image in that case, too, although it probably doesn't matter too much; there are ways of adding extra packages to the pbuilder image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need an answer for this; does clang-tidy exist for RHEL-like systems? or do we want to restrict this job to only apt systems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is available but for now lets keep the job to apt systems only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this accomplished? won't the command be executed (and fail) on all systems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The job specifies the labels for the jenkins builder it wants; we'll need to add something like 'jammy' as a label, that should suffice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where should I add this 'jammy' label?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
job.node is the list of labels the builder must satisfy. You probably want jammy && small
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmick I have added
node: jammy && small