Skip to content

test: initialize p_ops in test_lane_hold_release - #51

Merged
janekmi merged 1 commit into
masterfrom
osalyk/fix-clang-version
Nov 5, 2026
Merged

janekmi merged 1 commit into
masterfrom
osalyk/fix-clang-version

Conversation

@osalyk

@osalyk osalyk commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

This change is Reviewable

@osalyk
osalyk requested review from grom72 and janekmi November 3, 2026 12:09
@osalyk osalyk changed the title common: stop requiring clang-format-14 package Fix clang-format issues and update test for newer clang versions Nov 4, 2026

@janekmi janekmi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@janekmi reviewed 4 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on osalyk).


.github/workflows/main.yml line 43 at r1 (raw file):

        run: |
          sudo pip install flake8
          sudo apt-get install clang-format

Probably not needed at all right now.

Suggestion:

          sudo apt-get install

utils/style_check.sh line 90 at r1 (raw file):

		return
	fi
	check_clang_version

Note this line is probably never reached anyway since I would expect there are no arguments for clang-format so this function exits early.


utils/style_check.sh line 91 at r1 (raw file):

		*.[ch]pp)
			CLANG_ARGS+="${ARGS[$i]} "
			;;

Since you excluded all C++ sources from cstyle we can either remove all of these utilites or none of them. I would argue if you decide to leave them I would also leave check_clang_version since when in use we would probably would like to have control over the clang-format version.

Signed-off-by: Oksana Salyk <oksana.salyk@hpe.com>
@osalyk
osalyk force-pushed the osalyk/fix-clang-version branch from fd8970c to e82f911 Compare November 5, 2026 07:44
@osalyk osalyk changed the title Fix clang-format issues and update test for newer clang versions test: initialize p_ops in test_lane_hold_release Nov 5, 2026

@osalyk osalyk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@osalyk made 3 comments.
Reviewable status: 1 of 4 files reviewed, 3 unresolved discussions (waiting on janekmi).


.github/workflows/main.yml line 43 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Probably not needed at all right now.

All changes in #52


utils/style_check.sh line 90 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Note this line is probably never reached anyway since I would expect there are no arguments for clang-format so this function exits early.

.


utils/style_check.sh line 91 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Since you excluded all C++ sources from cstyle we can either remove all of these utilites or none of them. I would argue if you decide to leave them I would also leave check_clang_version since when in use we would probably would like to have control over the clang-format version.

.

@janekmi janekmi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm:

@janekmi reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on osalyk).

@janekmi
janekmi merged commit 755ce02 into master Nov 5, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants