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

Allow unit test rules to be documented #343

Merged
merged 3 commits into from
Feb 10, 2022
Merged

Allow unit test rules to be documented #343

merged 3 commits into from
Feb 10, 2022

Conversation

UebelAndre
Copy link
Contributor

I find it more ergonomic to add notes or a description about a test to a doc attribute, similar to other rules, vs a comment block above it or in the impl function docstring. Hopefully this can improve the readability of test rules.

@UebelAndre
Copy link
Contributor Author

@tetromino Friendly ping 😅

@UebelAndre
Copy link
Contributor Author

@tetromino One more friendly ping 😄 It'd be nice to have this in the next release

Copy link
Collaborator

@tetromino tetromino left a comment

Choose a reason for hiding this comment

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

Seems reasonable. Sorry for overlooking the PR!

@tetromino tetromino merged commit 2bc90bd into bazelbuild:main Feb 10, 2022
@UebelAndre UebelAndre deleted the doc branch February 10, 2022 22:16
@tetromino
Copy link
Collaborator

tetromino commented Feb 10, 2022

On second thought - we made a mistake here. New parameter should go to the end of the list, otherwise we break users who use analysistes.make with positional parameters and expect parameter 2 to be the expect_failure bool. Quick fix PR incoming, please review - sorry, turns out you don't have privileges to review - I need to find someone else.

tetromino pushed a commit that referenced this pull request Feb 10, 2022
ngiloq6 added a commit to ngiloq6/bazel-skylib that referenced this pull request Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants