-
Notifications
You must be signed in to change notification settings - Fork 452
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
Add a script to manage marker traits #793
Conversation
@BurntSushi Let me know if this is somewhat near what you had in mind. |
@BurntSushi Sorry to bother, is something blocking this PR, or is it way too different from what you expected? 🥺 |
Haven't had a chance to look yet, sorry. I have a baby at home and the past few weeks have been hectic. |
@BurntSushi Oh, no worries at all. Congrats!!! Sorry for bothering 😅 |
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.
OK, so I've finally had a chance to look at this. I also re-read #756, and I feel like there was a miscommunication perhaps. In particular, I specifically do not want extra automation for generating the tests. It's effectively extra code for almost no value. The public API just does not change much. So adding a new test by hand is done once in a blue moon and it doesn't cost much to do.
The only thing I really want here is something to tell me whether a new test needs to be added or not, and then run that in CI. I see that here, which is great. But I really think it should just be a shell script. I have almost gotten rid of Python from this repo, so I'd rather not bring more of it in.
I realize this is a really tardy review, so feel free to just drop this. The baby has kept me very busy. But if you're looking to keep going here, I'd say:
- Drop all the codegen.
- Convert the existence check to a simple shell script.
I'm going to close this out due to inactivity, but I would still like something simple that checks that we're covering the API with tests for marker traits. |
Closes #756.
This PR introduces a Python script named
public-types-tests.py
that manages marker traits for the public API types. Following the discussion in #756, this script is responsible for two tasks:writing the assertions to the file
tests/marker_traits.rs
. This shouldn't have to be done very often, probably just once. We pass the-g
flag as an argument to the script to generate the tests.This accomplishes three objectives:
Two things remain for this PR to be ready: