-
Notifications
You must be signed in to change notification settings - Fork 147
feat(checklists): add EIPChecklist
enum to specify EIP checklist items
#1718
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
Conversation
36ad5a4
to
3ad8a32
Compare
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.
The nested classes are insane 😄 but LGTM, thanks!
EIPChecklist
enum to access EIP Checklist ItemsEIPChecklist
enum to specify EIP checklist items
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.
Something's not quite right when running fill
.
Issue 1: Marker Without Parentheses Not WorkingProblem: Using Root Cause: The original implementation used Solution: Moved the marker logic from
Issue 2: MyPy Type Checking ErrorsProblem: MyPy showed Root Cause: MyPy doesn't understand metaclass magic that makes classes callable at runtime. The Solution: Created a comprehensive |
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.
Yo! LGTM! Thanks, the IDE auto-completion really improves UX!
5f1fc4f
to
89d1cf3
Compare
Update test files to use the correct checklist IDs that match the template: - Change 'new_precompile' prefix to 'precompile' - This fixes mkdocs build warnings about missing checklist items The checklist template uses 'precompile/test/*' IDs but tests were using 'new_precompile/test/*' IDs, causing template lookup failures during documentation generation. Also add missing docstrings to fix linting errors.
89d1cf3
to
c4bb1fa
Compare
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.
Hey @marioevz, I just fixed-up the CI fails due to incorrect eip checklist ids (hopefully 🤞).
I didn't change the string ID format to the new enum format to avoid conflicts; they're probably best solved in the branches working on these new tests.
Well at least to reduce them, if all the test PRs are merged we can of course address them here! |
🗒️ Description
This PR introduces the
EIPChecklist
class which contains a list of all EIP checklist items for easier marking of tests that cover an item in the EIP checklist.Calling any value in this class directly returns the marker which can be used in the tests or
marks
parameter of the parametrize object:It also fixes a breaking bug in the checklist generation code that marked all EIPs for a given test id when a marker was used anywhere in any test in any folder (whoops).
Suggestions for improvement of the usage of this new class are welcome.
🔗 Related Issues
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.