-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix missing node normalization to match pytree.convert #30
fix missing node normalization to match pytree.convert #30
Conversation
6ac2c24
to
cb35c00
Compare
It looks like the workflow needs to be manually enabled again https://docs.github.com/en/actions/managing-workflow-runs/disabling-and-enabling-a-workflow#disabling-and-enabling-workflows-with-the-github-ui |
Done that, can you amend your last commit to retrigger? |
664ecd5
to
8198858
Compare
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 77.60% 78.34% +0.74%
==========================================
Files 4 4
Lines 942 956 +14
Branches 214 219 +5
==========================================
+ Hits 731 749 +18
+ Misses 170 167 -3
+ Partials 41 40 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Thanks, fixing #25 was easier than I thought and didn't require as many changes to the dispatch functions as I initially had thought |
.github/workflows/check.yml
Outdated
@@ -2,8 +2,6 @@ name: check | |||
on: | |||
push: | |||
pull_request: | |||
schedule: |
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.
Any reason to remove these?
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 can back out the change, but I was wondering if that would fix the workflow being disabled 🤷
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.
Let's make it part of another PR if we'd do it.
8198858
to
652fa80
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.
Seems the pypi token we have set up is invalid, will reach out to @ambv to fix that and do a release after. Thanks for your contribution! |
No prob, thanks for the quick review. I have another PR that I was working on that will follow this, but it's possibly something that will take more time to review so I figured I'd fix the bugs I was running into first. It so happened they were already reported 😅 |
Release now out, all good! |
I was going to write this as a bug/question, but then I realized what was going on (I had already stepped through the lib2to3 code in PDB). It looks like this issue is the same as #23 and I added it's example and the test case I was going off of.
I will probably look at #25 since that looks like it's probably related to
convert_annotation(Attribute)
, but I'm not sure if it would be best to merge them together or split them out separately since they will likely be touching much of the same code.fixes: #23, fixes #25