Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Cleanup buildifer formatting and lint warnings. #84

Closed
wants to merge 1 commit into from

Conversation

chandlerc
Copy link
Contributor

@chandlerc chandlerc commented Nov 7, 2020

These aren't all trivial -- several of these showed questionable globs
that I've tried to fix in the way that seemed most effective.


This change is Reviewable

These aren't all trivial -- several of these showed questionable globs
that I've tried to fix in the way that seemed most effective.
@@ -2,6 +2,7 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need to add rules_cc to the workspace for this to work? https://github.com/bazelbuild/rules_cc#getting-started

It's not clear to me whether migrating to rules_cc is actually recommended right now. Internally I think we still use the native rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works for me without anything else.

And buildifier has a lint complaint w/o it that this fixes. So for example, without this, VSCode thinks that all of our uses of the native rules are issues. It highlights them, etc etc.

I don't see how this can not be recommended with the buildifier tool linting it for the last several months?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I am also confused. I've filed bazelbuild/rules_cc#92 about the documentation/tooling mismatch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any harm in landing this to silence the lint warnings in the interim?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea :-D There's at least some indication that the future of rules_cc is unclear and not well documented: bazelbuild/rules_cc#86. I'm a bit hesitant to move to it, given that

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants