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

deps: update bazel & bazel libs #4426

Merged
merged 8 commits into from
Oct 31, 2023

Conversation

lukedirtwalker
Copy link
Collaborator

@lukedirtwalker lukedirtwalker commented Oct 30, 2023

Drop the go_embed_data custom rules, those were deprecated for a long time.
Drop most of the custom rules_openapi bazel rules.
Users are recommended to use their own invocations of the JS tools.


This change is Reviewable

Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @lukedirtwalker)

Also drop the go_embed_data custom rules, those were deprecated for a long time.
Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewed 43 of 43 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @lukedirtwalker)

Copy link
Contributor

@oncilla oncilla left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r3, 15 of 15 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @lukedirtwalker)

Copy link
Contributor

@matzf matzf left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r1, 42 of 43 files at r2, 3 of 3 files at r3, 15 of 15 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @lukedirtwalker)


private/mgmtapi/api.bzl line 28 at r4 (raw file):

        srcs = [src],
        outs = [out],
        args = ["build-docs", "--output", "../../../$@", "../../../$(location {})".format(src)],

What are these paths? Can we instead change the working directory?


spec/ca/BUILD.bazel line 9 at r4 (raw file):

)

copy_to_bin(

Just out of curiosity; what is the difference to a filegroup here?

Copy link
Collaborator Author

@lukedirtwalker lukedirtwalker left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @lukedirtwalker)


private/mgmtapi/api.bzl line 28 at r4 (raw file):

Previously, matzf (Matthias Frei) wrote…

What are these paths? Can we instead change the working directory?

https://docs.aspect.build/guides/rules_js_migration/#account-for-change-to-working-directory It's needed because rules_js is a bit awkward in that respect. I don't think we could change directory.


spec/ca/BUILD.bazel line 9 at r4 (raw file):

Previously, matzf (Matthias Frei) wrote…

Just out of curiosity; what is the difference to a filegroup here?

It's required to copy the files to bin dir for rules_js. See https://docs.aspect.build/rulesets/aspect_rules_js#running-nodejs-programs for more details (the last bullet point)

@lukedirtwalker lukedirtwalker merged commit 9ebc890 into scionproto:master Oct 31, 2023
1 check passed
@lukedirtwalker lukedirtwalker deleted the deps-upgrade branch October 31, 2023 12:31
juagargi pushed a commit to netsec-ethz/scion that referenced this pull request Mar 8, 2024
Drop the `go_embed_data` custom rules, those were deprecated for a long time.
Drop most of the custom `rules_openapi` bazel rules.
Users are recommended to use their own invocations of the JS tools.
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