-
Couldn't load subscription status.
- Fork 185
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! |
Also removes a file that was unintentionally added in #1718.
Also removes a file that was unintentionally added in #1718.
…ems (ethereum#1718) * fix: move unit test * Create Enums - Incomplete/broken attempt * fix(filler/checklist): Template copying bug * fix: EIPChecklist usage * docs: Update usage * refactor(eip_checklist): Remove new_* from all IDs * fix: tox (minus spellcheck) * fix: typing * fix(docs,checklists): Rename section to be more descriptive * docs: remove references to string EIP checklist notation * docs: backtick `EIPChecklist` instances * docs: update changelog * docs: fix title * fix(checklists): allow ommission of parentheses in checklist markers * style(checklists): add stubs to solve mypy issues using checklist markers * test(checkmarks): add fw tests that apply EIPChecklists in pytest.param * fix(docs): fix bad links to exception tests * fix(checklists): update test markers to use correct checklist IDs 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. --------- Co-authored-by: danceratopz <[email protected]>
🗒️ Description
This PR introduces the
EIPChecklistclass 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
marksparameter 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 servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.