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.

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

feat(dev): add changelog check into pre-commit #7377

Open
wants to merge 10 commits into
base: 4.x
Choose a base branch
from
34 changes: 17 additions & 17 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@ Fixes #(issue)

<!-- Please delete options that are not relevant. -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

## Checklist:

- [ ] I have selected the correct base branch.
- [ ] I have performed a self-review of my own code.
- [ ] I have commented my code, particularly in hard-to-understand areas.
- [ ] I have made corresponding changes to the documentation.
- [ ] My changes generate no new warnings.
- [ ] Any dependent changes have been merged and published in downstream modules.
- [ ] I ran `npm run lint` with success and extended the tests and types if necessary.
- [ ] I ran `npm run test:unit` with success.
- [ ] I ran `npm run test:coverage` and my test cases cover all the lines and branches of the added code.
- [ ] I ran `npm run build` and tested `dist/web3.min.js` in a browser.
- [ ] I have tested my code on the live network.
- [ ] I have checked the Deploy Preview and it looks correct.
- [ ] I have updated the `CHANGELOG.md` file in the root folder.
- [ ] I have linked Issue(s) with this PR in "Linked Issues" menu.
- [ ] I have selected the correct base branch.
- [ ] I have performed a self-review of my own code.
- [ ] I have commented my code, particularly in hard-to-understand areas.
- [ ] I have made corresponding changes to the documentation.
- [ ] My changes generate no new warnings.
- [ ] Any dependent changes have been merged and published in downstream modules.
- [ ] I ran `npm run lint` with success and extended the tests and types if necessary.
- [ ] I ran `npm run test:unit` with success.
- [ ] I ran `npm run test:coverage` and my test cases cover all the lines and branches of the added code.
- [ ] I ran `npm run build` and tested `dist/web3.min.js` in a browser.
- [ ] I have tested my code on the live network.
- [ ] I have checked the Deploy Preview and it looks correct.
- [ ] I have updated the `CHANGELOG.md` file in both the root folder and corresponding packages/web3-xxx folders.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These spaces are added automatically by the check

- [ ] I have linked Issue(s) with this PR in "Linked Issues" menu.
2 changes: 2 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
. "$(dirname -- "$0")/_/husky.sh"

npx lint-staged

npm run check-changelog
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@
"postinstall": "yarn build",
"compile:contracts": "node ./scripts/compile_contracts.js && yarn format && yarn lint:fix",
"publish:canary": "lerna publish --canary --dist-tag dev --preid dev.$(git rev-parse --short HEAD) --exact --graph-type all --force-publish \"*\" --no-verify-access --yes",
"prepare": "husky install"
"prepare": "husky install",
"check-changelog": "node ./scripts/check_changelog.js"
},
"devDependencies": {
"@cypress/webpack-preprocessor": "^5.12.0",
Expand Down
88 changes: 88 additions & 0 deletions scripts/check_changelog.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
const { execSync } = require('child_process');
const path = require('path');
const fs = require('fs');

// Colors for output
const colors = {
red: text => `\x1b[31m${text}\x1b[0m`,
green: text => `\x1b[32m${text}\x1b[0m`,
};

try {
// Get all staged files
const stagedFiles = execSync('git diff --cached --name-only', { encoding: 'utf-8' })
.trim()
.split('\n')
.filter(Boolean);

// Initialize map to track packages with code changes and changelog updates
const packagesWithChanges = new Map();

// Scan all staged files
stagedFiles.forEach(file => {
// Get package name
const packageMatch = file.match(/packages\/([^/]+)/);
if (!packageMatch) return;

const packageName = packageMatch[1];

// Check if it's a code file
const isCodeFile = /\.(js|jsx|ts|tsx|css|json)$/.test(file);
whitemoshui marked this conversation as resolved.
Show resolved Hide resolved
// Check if it's a changelog file
const isChangelog = file.endsWith('CHANGELOG.md');

if (!packagesWithChanges.has(packageName)) {
packagesWithChanges.set(packageName, {
hasCodeChanges: false,
hasChangelogUpdate: false,
});
}

const packageInfo = packagesWithChanges.get(packageName);

if (isCodeFile && !isChangelog) {
packageInfo.hasCodeChanges = true;
}

if (isChangelog) {
packageInfo.hasChangelogUpdate = true;
}
});

// Check if packages with code changes have changelog updates
let hasError = false;

for (const [packageName, info] of packagesWithChanges) {
if (info.hasCodeChanges) {
if (!info.hasChangelogUpdate) {
console.log(
colors.red(
`Error: Package '${packageName}' has code changes but no CHANGELOG.md update`,
),
);
hasError = true;
} else {
console.log(
colors.green(
`✓ Package '${packageName}' has both code changes and CHANGELOG.md update`,
),
);
}
}
}

if (hasError) {
console.log(
colors.red(
'\nCommit rejected: Please update packages/web3-xxx/CHANGELOG.md for all modified packages',
Copy link
Contributor

Choose a reason for hiding this comment

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

would be better to name the actual package rather web3-xxx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The log above will display the specific package names. Maybe something like "Please update the changelog for all aforementioned packages." is enough? I agree that web3-xxx is not clear enough

),
);
process.exit(1);
} else if (packagesWithChanges.size > 0) {
console.log(colors.green('\nAll package changes have corresponding changelog updates'));
}
} catch (error) {
console.error(colors.red('Error executing script:'), error);
// Skip this check and return 0 if something goes wrong
process.exit(0);
}