Skip to content

style: Declare data of Stream classes private, instead of protected - #1422

Merged
thewtex merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Declare-Stream-data-private
Jul 18, 2025
Merged

style: Declare data of Stream classes private, instead of protected#1422
thewtex merged 1 commit into
InsightSoftwareConsortium:mainfrom
N-Dekker:Declare-Stream-data-private

Conversation

@N-Dekker

Copy link
Copy Markdown
Contributor

Following C++ Core Guidelines, Jul 8, 2025, "Avoid protected data"

@N-Dekker

Copy link
Copy Markdown
Contributor Author

@thewtex The CI rejects this PR at PR Checks / Check and lint PR (pull_request), saying:

   type must be lower-case [type-case]
   type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] [type-enum]

Could this commit check possibly be extended, to allow ITK style commit text?

@N-Dekker
N-Dekker force-pushed the Declare-Stream-data-private branch from 733b6fd to 0fb1638 Compare July 18, 2025 12:45
@thewtex

thewtex commented Jul 18, 2025

Copy link
Copy Markdown
Member

👍 @N-Dekker thank you!

type must be lower-case [type-case]
type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] [type-enum]

These are the conventions of a broadly adopted standard, Conventional Commit:

https://www.conventionalcommits.org/en/v1.0.0/

that is used throughout the web community, and has a bunch of tooling built around it. Since this is a web-oriented project and we use some of that tooling, we use this convention.

@N-Dekker N-Dekker changed the title STYLE: Declare data of Stream classes private, instead of protected style: Declare data of Stream classes private, instead of protected Jul 18, 2025

@thewtex thewtex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🙏

@thewtex
thewtex merged commit 3818ff2 into InsightSoftwareConsortium:main Jul 18, 2025
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants