-
Notifications
You must be signed in to change notification settings - Fork 242
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
Make the switch to docc
#833
Conversation
611357b
to
2c9d1af
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.
I haven't reviewed docc itself, but this all looks good.
Update doc build to use docc Update pyproject.toml Update gh-pages.yaml Start a docc plugin for diffs Fix build inside of tox Get diff working Use ins/del instead of div Reduce irrelevant diffs in name nodes Pass through parent instead of creating div Support markdown diff pull in latest fladrif Group diffs where possible Show diffs in directory listings Exclude ethereum_{optimized,spec_tools} Switch to published docc
For some reason, pypy (and cpython on GitHub) cannot seem to correctly patch the london hardfork using the `patch` function. I've switched to using the `patch.object` function since you can explicitly provide the object, bypassing the import error. I'm not 100% sure this is correct.
fork_module = importlib.import_module( | ||
f"ethereum.{load.fork_module}.fork" | ||
) | ||
with patch.object( | ||
fork_module, | ||
"validate_proof_of_work", |
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.
@gurukamath @petertdavies if you could double check that this change maintains the original behaviour, I'd appreciate it!
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #833 +/- ##
==========================================
+ Coverage 74.10% 74.11% +0.01%
==========================================
Files 572 572
Lines 32028 32411 +383
==========================================
+ Hits 23733 24022 +289
- Misses 8295 8389 +94
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b13c22b
to
ffd6e1f
Compare
ec0cc75
to
3de7833
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.
LGTM!
if isinstance(pkg.module_finder, importlib.abc.MetaPathFinder): | ||
found = pkg.module_finder.find_module(pkg.name, None) | ||
else: | ||
found = pkg.module_finder.find_module(pkg.name) | ||
|
||
if not found: | ||
raise Exception(f"unable to load module {pkg.name}") | ||
|
||
mod = found.load_module(pkg.name) |
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.
@SamWilsn After some investigation, I have narrowed it down and it seems to me that these changes (L88 - L96) are causing a memory bloat while loading and running evm tools tests. Not entirely sure why though. Would you have any thoughts?
What changed?
This rips out sphinx and replaces it with docc.
Opening this as a draft because I'm still waiting on a few upstream PRs to merge.
Review Suggestions
I very strongly suggest doing this review one commit at a time. Several commits are just fixing formatting/flake8 problems that came up after updating the packages.
Cute Animal Picture