-
Notifications
You must be signed in to change notification settings - Fork 6
bazel: remove unused dependencies #21
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
base: main
Are you sure you want to change the base?
Conversation
|
The created documentation from the pull request is available at: docu-html |
d4043d4 to
4944ae7
Compare
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
4944ae7 to
c3386cd
Compare
|
@rmaddikery, @hoppe-and-dreams please review |
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| load("@rules_cc//cc:cc_library.bzl", "cc_library") |
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.
The commit and changes do not reflect intent. Also please clarify why this change is made/required
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.
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.
Forked PR: #24
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.
Use Bazel 8+ managed via https://bazel.build/install/bazelisk
Then try the following:
$ bazel build //score/datarouter:datarouter
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.
@rmaddikery Please take a look into official C++ bazel example - https://github.com/bazelbuild/examples/blob/main/cpp-tutorial/stage1/main/BUILD
First versions of bazel did not require load statement for cc_binary / library but since 2019, there is a hint to use it as it will be a future incompatibility already.
c3386cd to
450e1de
Compare
450e1de to
8e19335
Compare
score/datarouter/BUILD
Outdated
| # SPDX-License-Identifier: Apache-2.0 | ||
| # ******************************************************************************* | ||
|
|
||
| load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") |
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.
rmaddikery
left a comment
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.
Changes in 8e19335 is fine. If you exclude the other commit, we can merge the PR.
Reasoning: #24 (comment)
- Remove unused dependencies (non-exhaustive). - Remove unused `third_party` files.
8e19335 to
823d938
Compare
rmaddikery
left a comment
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.
LGTM

third_partyfiles.Resolves #19