Skip to content

feat: add code quality check script and license config#151

Closed
discivigour wants to merge 1 commit intoapache:mainfrom
discivigour:add-check-script
Closed

feat: add code quality check script and license config#151
discivigour wants to merge 1 commit intoapache:mainfrom
discivigour:add-check-script

Conversation

@discivigour
Copy link
Copy Markdown
Contributor

Purpose

Add code quality check script and license config.

Brief change log

Tests

API and Format

  • ./dev/check.sh

Documentation

Comment thread dev/check.sh
echo ""
echo "[1/4] Checking license headers..."
if command -v license-eye &> /dev/null; then
license-eye header check
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.

This project already uses skywalking-eyes to check for licenses.

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.

Where is the skywalking-eyes? You mean ci.yml? I added this script to facilitate local verification.

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.

Where is the skywalking-eyes? You mean ci.yml? I added this script to facilitate local verification.

Nice initiative! For the "facilitate local verification" goal, a pre-commit hook might be more appropriate — it runs automatically on every commit, so contributors can't forget. Consider adding a .pre-commit-config.yaml for fmt/clippy checks. The Makefile targets and .licenserc.yaml are still valuable and can be kept. The dev/check.sh script could be simplified or removed if pre-commit covers the same ground.

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.

Thanks, I will try it.

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.

Thanks for the suggestion! I did look into pre-commit hooks and .pre-commit-config.yaml before going with dev/check.sh.

I don't think it's necessary to introduce git hooks at present. Because it cannot be uploaded to the repository, or relies on external plugins, or is manually configured, and the current repository is not very large at present.

dev/check.sh provides an easy, zero-dependency way for contributors to verify locally, and CI serves as the final safeguard. I think this combination is sufficient for the current project scale without introducing additional tooling overhead. We can revisit this if the project grows and the team scales.

@JingsongLi
Copy link
Copy Markdown
Contributor

It seems that we have already checked and there may be some duplicates in this PR. I will close it first and we can bring up a new PR.

@JingsongLi JingsongLi closed this Apr 1, 2026
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.

3 participants