-
Notifications
You must be signed in to change notification settings - Fork 592
[email protected] #6510
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
[email protected] #6510
Conversation
|
Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (nextpnr, prjtrellis) have been updated in this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds two new modules, [email protected] and its dependency [email protected]. The overall structure is good and follows the BCR guidelines. However, I've found a few critical configuration errors that need to be addressed:
- The module name for
nextpnris misspelled asnexpnrin itsMODULE.bazelfiles. - The
source.jsonfor theprjtrellismodule incorrectly points to thenextpnrsource archive.
Please see the detailed comments for suggestions on how to fix these issues.
|
@bazel-io skip_check unstable_url |
|
@fmeum can I get a presubmit stamp? |
| http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") | ||
|
|
||
| http_archive( | ||
| name = "prjtrellis_db", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a module or is it okay if this is duplicated across multiple deps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Converted this to a module!
Head branch was pushed to by a user without write access
|
Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (nextpnr, prjtrellis-db, prjtrellis) have been updated in this PR. |
|
@fmeum friendly ping here. |
closes #6065