prjtrellis@0.0.0-20251212-e076bca#6801
Conversation
|
Hello @UebelAndre, modules you maintain (prjtrellis) have been updated in this PR. |
There was a problem hiding this comment.
Code Review
This pull request adds the version 0.0.0-20251212-e076bca for the prjtrellis module. While the new version's files (MODULE.bazel, presubmit.yml, etc.) appear to be correctly structured, this PR introduces a critical issue by modifying modules/prjtrellis/metadata.json. It changes the module's homepage, maintainer, and source repository to point to a fork. This is a significant change that effectively transfers ownership of the module and violates the Bazel Central Registry's 'add-only' policy for non-versioned files. Such a change requires manual review and approval from BCR maintainers. If the intention is to publish a fork as a separate module, it should be given a distinct name.
| "homepage": "https://github.com/MrAMS/prjtrellis", | ||
| "maintainers": [ | ||
| { | ||
| "email": "26427366+UebelAndre@users.noreply.github.com", | ||
| "github": "UebelAndre", | ||
| "github_user_id": 26427366, | ||
| "name": "UebelAndre" | ||
| "email": "", | ||
| "github": "MrAMS", | ||
| "name": "Qijia Yang", | ||
| "github_user_id": 25056812 | ||
| } | ||
| ], | ||
| "repository": [ | ||
| "github:YosysHQ/prjtrellis" | ||
| "github:MrAMS/prjtrellis" | ||
| ], |
There was a problem hiding this comment.
This pull request modifies the homepage, maintainers, and repository fields for the existing prjtrellis module, effectively transferring ownership from the original YosysHQ/prjtrellis to the MrAMS/prjtrellis fork. This is a significant and potentially breaking change for existing users of this module.
This change violates two key BCR policies:
- Add-only rule (line 13): Pull requests should be 'add-only' and not mutate non-module files when adding a new version. Modifying
metadata.jsonto change ownership affects all versions of the module. - Significant changes review (line 62): The style guide requires a manual review by
@bazelbuild/bcr-maintainersfor any significant changes to themaintainersorrepositoryfields. This change from an organization repo to a personal fork is a very significant change.
If this is a fork that should coexist with the original, it should be published under a new module name (e.g., mrams-prjtrellis). If this is an intentional transfer of ownership, it requires explicit approval from the original maintainers and the BCR team.
References
- Per line 13 of the style guide: PRs should be 'add-only' and not mutate non-module files (like metadata.json) when adding a new version. This PR modifies the
homepage,maintainers, andrepositoryfields, which affects the entire module. (link) - Per line 62 of the style guide: Significant changes to
maintainersorrepositoryfields inmetadata.jsonrequire a manual review by BCR maintainers. This change fromYosysHQ/prjtrellisto a fork is a significant change. (link)
|
This PR contributes to the full implementation of the rules_hdl migration. Compared to the previous version, this update enhances the BCR smoke tests and resolves dependency issues. Additionally, I have configured a monthly CI workflow to automatically fetch and merge updates from the upstream repository. I look forward to any feedback from the reviewers. ❤️ |
UebelAndre
left a comment
There was a problem hiding this comment.
This thread must first be resolved: hdl/bazel_rules_hdl#428 (comment)
|
This PR has been automatically marked as stale because it has not had any activity for 30 days. It will be closed in 14 days if no further activity occurs. Please comment on this PR to keep it open, or ask |
Release: https://github.com/MrAMS/prjtrellis/releases/tag/v0.0.0-20251212-e076bca
Automated by Publish to BCR