-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fewer words, easier reading #9326
Conversation
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
Oops, can you double-check? I don't see the expected changes |
7ec597a
to
4a714a3
Compare
My bad! changes are now in. Thank you! |
site/docs/skylark/language.md
Outdated
@@ -105,10 +105,9 @@ illegal, and 2) `*args` and `**kwargs` arguments are not allowed. | |||
* Global variables are immutable. | |||
|
|||
* `for` statements are not allowed at the top-level. Use them within functions | |||
instead. In BUILD files, you may use list comprehensions. | |||
instead. In BUILD files, you may use list comprehensions. That said, we prefer that you use macros, because we think they are easier to read. |
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.
I'd remove this line (I know it was suggested by @brandjon).
We also want to limit the use of macros.
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.
I removed it. Thank you!
incorporating feedback incorporating feedback applying fixes applying fixes
4a714a3
to
a5f63b1
Compare
Incorporating feedback from PR 9310.