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

Design a way to emit warnings without spamming the user #4772

Open
rosun82 opened this issue Mar 5, 2018 · 8 comments
Open

Design a way to emit warnings without spamming the user #4772

rosun82 opened this issue Mar 5, 2018 · 8 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols type: feature request

Comments

@rosun82
Copy link

rosun82 commented Mar 5, 2018

Description of the problem / feature request:

Currently skylark provides a "print" function, which produce a line prefixed with "DEBUG". It is often useful to print a line with prefix "WARNING", however, this is no such function right now.

@laszlocsomor laszlocsomor added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) category: extensibility > skylark labels Mar 6, 2018
@laszlocsomor
Copy link
Contributor

/cc @vladmos

@laurentlb
Copy link
Contributor

Hi,

Thanks for the feedback. You're not the first person to ask for it, but we have to be careful.

  • The person who sees the warning is not necessarily the person who can fix it.
  • When warnings accumulate on the output, users will start ignoring them. They will perceive the output stream as spammy. They will not notice when a new warning was introduced.

I think it's hard to do warnings correctly based on dynamic information. Imagine you want to warn about a specific value your function received. How to know who is responsible for it? How can we know where the value comes from exactly? The immediate caller of your function is not necessarily responsible for the value.

I think warnings based on static information are more reliable in general. As a proof of concept, we have a linter that can detect calls to deprecated functions: https://github.com/bazelbuild/bazel/blob/master/site/docs/skylark/skylint.md#deprecating-functions-docstring-format-deprecated-symbol

My preference would be to develop more static checks like this, instead of adding a new function.

@pauldraper
Copy link
Contributor

pauldraper commented Sep 4, 2018

@laurentlb I love those ideas in theory.

I would also love those ideas in practice....if you could get the rules in Bazel core to follow them. (e.g. strict java deps).

@benjaminp
Copy link
Collaborator

I don't understand the relevance of strict Java deps. By default, those are errors not warnings.

@vmax
Copy link
Contributor

vmax commented Nov 1, 2018

A fine-ish approach would be to use colored print:

def warn(msg):
    print('{red}{msg}{nc}'.format(red='\033[0;31m', msg=msg, nc='\033[0m'))

@pauldraper
Copy link
Contributor

I don't understand the relevance of strict Java deps.

Strict Java deps uses warnings.

It shows a good example of when rule authors (in this case, the Java rule authors) needed warning capabilities.

@brandjon
Copy link
Member

I agree that warnings are generally spammy to the majority of people who will see them, and that we shouldn't encourage rule, macro, and repository rule authors to add more of them. I also agree that we could be doing more to provide better alternatives, whether those be static linters or opt-in filters of some sort.

I don't think we can prioritize this anytime soon though. Leaving the issue open for further design discussion.

@brandjon brandjon added team-Build-Language P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed team-Starlark P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Feb 16, 2021
@brandjon brandjon changed the title Adding a "warn" function to skylark Design a way to emit warnings without spamming the user Feb 16, 2021
@brandjon brandjon added team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols and removed team-Build-Language labels Nov 4, 2022
@celestialorb
Copy link

Wouldn't it make sense to have the ability to produce warnings if you're creating custom rules? What if I want to warn the user about potentially nonsensical inputs to a rule? I don't see how it would be different from sanity checking inputs to a function in another language.

What about deprecating inputs to a rule? I'd prefer to warn the user about the deprecation for some time before forcing an error. Without a way to produce a warning I'd have to create a new rule and have users migrate to that if I wanted to clean up a rule. Granted this shouldn't happen often, but it does happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants