Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add more linting and apply changes; Clean-up GH workflows. #4

Merged
merged 33 commits into from
Jun 4, 2024
Merged

Add more linting and apply changes; Clean-up GH workflows. #4

merged 33 commits into from
Jun 4, 2024

Conversation

davidlion
Copy link
Member

Description

The linting and formatting for c++ code had fallen behind CLP and the linting for go code was not yet in workflows. This PR updates the linting and formatting for both languages:

  • switch to golangci-lint for go linting and formatting checks
  • update clang-format configuration + source
  • update clang-tidy to pass on linux and mac
  • update workflows to properly run linting on linux and mac

Validation performed

Validated on github actions

davidlion added 25 commits June 2, 2024 16:38
@davidlion
Improve linting: add golangci, update go source, update workflows
ab712cc
@davidlion
Yaml lint/fixing
ae27a04
@davidlion
test workflow?
9e74a1e
@davidlion
forgot ubuntu
806864f
@davidlion
hmm
323b750
@davidlion
test
3626197
@davidlion
strings in strings
5d21ee0
@davidlion
finalize if statment; fix cpp lint
f1b1ffb
@davidlion
run llvm script as root
4a2e614
@davidlion
create compile commands.json for clang tidy
f4d6033
@davidlion
fix llvm/clang version to 18
f33900f
@davidlion
switch to pip llvm
4cfc078
@davidlion
fix clang 18 clp issue by changing clp headers to be included as SYST…
131ef7b 
…EM (-isystem) headers
@davidlion
try apt update
2d51ea7
@davidlion
change back clang install and try -18 suffix
1af9701
@davidlion
missing sudo
2d57ddd
@davidlion
brew doesn't add -18
6e1af4c
@davidlion
no shot
4d3ca8a
@davidlion
update clang-format and source to clp config
8896f4b
@davidlion
testing...
3663472
@davidlion
remove test
1f972b2
@davidlion
Revert accidental removal of mac packaged library in "fix clang 18 cl…
88328de 
…p issue by changing clp headers to be included as SYSTEM (-isystem) headers"

This reverts part of commit 131ef7b.
@davidlion
please github
9422c5d
@davidlion
switch project include to quotes from arrows
291a152
@davidlion
fix cgo header c/c++ interactions for linting
b9dc797
@davidlion
Copy link
Member Author

cc: @davemarco

Next PR should add yaml linting and taskfiles.

LinZhihao-723
LinZhihao-723 reviewed Jun 4, 2024
View reviewed changes
.github/workflows/lint.yml Outdated Show resolved Hide resolved
ir/deserializer.go Show resolved Hide resolved
kirkrodrigues
kirkrodrigues requested changes Jun 4, 2024
View reviewed changes
.github/workflows/test.yml Outdated Show resolved Hide resolved
.github/workflows/test.yml Outdated Show resolved Hide resolved
cpp/CMakeLists.txt Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
kirkrodrigues
kirkrodrigues approved these changes Jun 4, 2024
View reviewed changes
Copy link
Member

@kirkrodrigues kirkrodrigues left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the PR title, how about:

Add more linting and apply changes; Clean-up GH workflows.

@davidlion davidlion merged commit 969c115 into y-scope:main Jun 4, 2024
8 checks passed
@kirkrodrigues kirkrodrigues changed the title Update linting and formatting: update configs and workflows, fix source files Add more linting and apply changes; Clean-up GH workflows. Jun 4, 2024
@davidlion davidlion deleted the linting branch June 18, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LinZhihao-723 LinZhihao-723 LinZhihao-723 left review comments

@kirkrodrigues kirkrodrigues kirkrodrigues approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidlion @kirkrodrigues @LinZhihao-723