bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Warning message not consistent with BULD Style Guide

Open limdor opened this issue 3 years ago • 4 comments

Description of the problem / feature request:

In the BUILD Style Guide in the section other conventions you can find the following text:

When setting boolean-type attributes, use boolean values, not integer values. For legacy reasons, rules will still convert integers to booleans as needed, but this is discouraged. Rationale: flaky = 1 could be misread as saying “deflake this target by rerunning it once”. flaky = True unambiguously says “this test is flaky”.

However in some cases Bazel prints a message like: setting 'linkstatic=1' is recommended if there are no object files

This feels kind of contradictory with the recommendation and it would be expected to be linkstatic=True

Feature requests: what underlying problem are you trying to solve with this feature?

Get a clear and consistent style guide for BUILD files

What's the output of bazel info release?

4.1.0

Have you found anything relevant by searching the web?

BUILD Style Guide in the section other conventions We had already an initial discussion about the topic with @oquenchil

Any other information, logs, or outputs that you want to share?

I prepared these two PRs to try to fix it: https://github.com/bazelbuild/bazel/pull/13583 https://github.com/bazelbuild/bazel/pull/13584

But the outcome of the discussion was that if done it would be preferred to just to just disallow the usage of integer in Boolean parameters. When creating the PRs I did not mention what the BUILD Style Guide was saying because I was not aware. But now seeing that is there I find it more relevant and I would like to have this ticket to start a discussion about it.

I would like to get to know if some other people think that the message is contradictory with the style guide or not.

limdor avatar Jun 26 '21 07:06 limdor

Previous discussion: https://github.com/bazelbuild/bazel/issues/4792

laurentlb avatar Jul 07 '21 07:07 laurentlb

Also: https://groups.google.com/g/bazel-discuss/c/M_LCQuLk-BQ/m/331pE6sXAwAJ?pli=1

Inside Google, there's also b/73722197

laurentlb avatar Jul 07 '21 07:07 laurentlb

We are closing this issue as we can see the above internal issue is fixed and also docs are updated accordingly. Please reach us back if you still have any query w.r.t it or reopen a new issue with same details as above.

sgowroji avatar Aug 02 '22 06:08 sgowroji

@sgowroji the style guide still discourages to use = 1 and there are still several places where the warning still suggests to use =1. https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java#L511 Could you just reopened this ticket instead of having to create a new one? With this way it is easier to keep the history.

limdor avatar Aug 02 '22 09:08 limdor