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 · 10 comments
Open

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

rosun82 opened this issue Mar 5, 2018 · 10 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.

Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Nov 18, 2024
@bluec0re
Copy link

Another use case I could think of: environmental differences (like host platform) which could lead to undesired effects (like cache misses), but don't affect the result. For example rules_pythons issue with the creation of pyc cache files by the python runtime on windows or builds with the root user.

It is something you probably want to know about if you build on different platforms, but it isn't something which should prevent you from building in the first place.

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Nov 26, 2024
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

9 participants