feat: enhance semantic-release path filter to support multiple paths …#154
Conversation
…and update package configurations.
There was a problem hiding this comment.
Pull request overview
This PR enhances the semantic-release path filter plugin to support monitoring multiple paths simultaneously, enabling packages in a monorepo to trigger releases based on changes in their dependencies. The implementation adds a new paths configuration option that works alongside the existing path option, and updates several package configurations to leverage this new capability.
Key Changes:
- Added
normalizePaths()function to combine and normalize bothpathandpathsconfiguration options - Refactored
filterCommitsByPath()tofilterCommitsByPaths()to handle multiple path prefixes - Fixed a critical bug in
packages/ducpdf/release.config.cjswhere the wrong package path was being versioned - Removed duplicate
block_idsinitialization in Python element builder
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/semrel-path-filter.cjs | Core implementation adding multi-path support with new normalizePaths() function and refactored filtering logic |
| packages/ducsvg/release.config.cjs | Configured to trigger releases on changes to ducsvg, ducpdf, ducjs, or ducrs packages |
| packages/ducpdf/src/duc2pdf/release.config.cjs | Added ducrs as an additional monitored path |
| packages/ducpdf/release.config.cjs | Added ducjs and ducrs as monitored paths; fixed prepareCmd to reference correct package |
| packages/ducdxf/release.config.cjs | Configured to monitor ducdxf and ducpy packages |
| packages/ducsvg/README.md | Added documentation note about potential future refactoring using hayro_svg |
| packages/ducpy/src/ducpy/builders/element_builders.py | Removed duplicate setdefault call for block_ids |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const filtered = await filterCommitsByPaths(context.commits, prefixes); | ||
|
|
||
| context.logger.log(`[path-filter] ${filtered.length}/${context.commits.length} commits affect "${pkgPath}"`); | ||
| const pathsDisplay = prefixes.map(p => p.slice(0, -1)).join(', '); |
There was a problem hiding this comment.
When no prefixes are configured (empty array), the log message will display an empty string between the quotes: commits affect "". This could be confusing. Consider adding a check to display a more meaningful message when no paths are configured, such as "all paths" or handling this case explicitly.
| const pathsDisplay = prefixes.map(p => p.slice(0, -1)).join(', '); | |
| const pathsDisplay = prefixes.length === 0 | |
| ? 'all paths' | |
| : prefixes.map(p => p.slice(0, -1)).join(', '); |
| path: "packages/ducpdf", | ||
| paths: [ | ||
| "packages/ducjs", | ||
| "packages/ducrs" |
There was a problem hiding this comment.
Missing trailing comma after the last array element "packages/ducrs". For consistency with other configuration files in this PR (ducsvg and ducdxf), add a trailing comma here.
| "packages/ducrs" | |
| "packages/ducrs", |
| function normalizePaths(pluginConfig) { | ||
| const { path, paths } = pluginConfig; | ||
|
|
||
| // Combine both options - paths array takes precedence but path is also included |
There was a problem hiding this comment.
The comment states "paths array takes precedence but path is also included" which is misleading. The current implementation doesn't show any precedence - both path and paths are combined equally. If paths was intended to take precedence (meaning it should override path when present), the logic should only use path when paths is not provided. Consider updating the comment to accurately reflect the implementation, such as "Combine both path and paths options, including all specified paths".
| // Combine both options - paths array takes precedence but path is also included | |
| // Combine both path and paths options, including all specified paths |
| function normalizePaths(pluginConfig) { | ||
| const { path, paths } = pluginConfig; | ||
|
|
||
| // Combine both options - paths array takes precedence but path is also included | ||
| const allPaths = []; | ||
|
|
||
| if (path) { | ||
| allPaths.push(path); | ||
| } | ||
|
|
||
| if (Array.isArray(paths)) { | ||
| allPaths.push(...paths); | ||
| } | ||
|
|
||
| // Remove duplicates and normalize | ||
| return [...new Set(allPaths)].map(normalizePrefix); |
There was a problem hiding this comment.
The API design allows both path and paths to be specified simultaneously, which creates an unclear interface. Users might expect paths to replace path when provided, but the implementation combines them. This could lead to unintended behavior where users specify paths thinking it will override path, but both are used. Consider either: (1) making them mutually exclusive with a validation warning, or (2) documenting this combination behavior very clearly in the function's JSDoc.
…ion to `paths` array and update script to only use `paths`.
|
🎉 This PR is included in version 2.3.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.1.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
…and update package configurations.