Skip to content
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

#160 list deserializer propagates fork_inst #161

Conversation

patrickguenther
Copy link
Contributor

@patrickguenther patrickguenther commented Apr 19, 2022

resolves #160
Other container serializers/deserializers don't seem to be affected by this bug, since they seem to already handle fork_inst correctly, though I haven't tested this.

@patrickguenther patrickguenther force-pushed the bugfix/default-list-deserializer-not-passing-fork-inst branch from 01f6333 to daf5fe9 Compare April 19, 2022 18:49
@patrickguenther patrickguenther changed the title list deserializer propagates fork_inst #160 list deserializer propagates fork_inst Apr 19, 2022
@patrickguenther patrickguenther force-pushed the bugfix/default-list-deserializer-not-passing-fork-inst branch from daf5fe9 to b4966fd Compare April 19, 2022 19:25
Seems that Python3.5 is no longer available on Actions. Can you turn it off so the checks can run on the other versions?
@patrickguenther
Copy link
Contributor Author

Those failed checks don't seem to be my fault, are they?

@ramonhagenaars
Copy link
Owner

Those failed checks don't seem to be my fault, are they?

No, it seems that Python3.5 can no longer be installed with Actions on an x64 architecture... Can you maybe turn off the checks for 3.5 to see if the checks at least succeed for recent Python versions?

@patrickguenther patrickguenther force-pushed the bugfix/default-list-deserializer-not-passing-fork-inst branch from b4966fd to bb21dff Compare April 25, 2022 10:14
@patrickguenther
Copy link
Contributor Author

It says, "1 workflow awaiting approval". Apparently, you have to sign off on workflow executions, for PRs from first time contributors.

Upgraded `setup-python` to `setup-pythonv3` to resolve the issues with python versions.
@codecov-commenter
Copy link

codecov-commenter commented May 10, 2022

Codecov Report

Merging #161 (ab5895c) into master (a5150cd) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #161   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           60        60           
  Lines         1488      1488           
=========================================
  Hits          1488      1488           
Impacted Files Coverage Δ
jsons/deserializers/default_list.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5150cd...ab5895c. Read the comment docs.

@ramonhagenaars
Copy link
Owner

All is green now. I had to upgrade actions/setup-python to actions/setup-python@v3 in the Github Actions workflow file.

Thank you @patrickguenther , I will gladly merge and release this!

@ramonhagenaars ramonhagenaars merged commit c2733eb into ramonhagenaars:master May 11, 2022
@patrickguenther patrickguenther deleted the bugfix/default-list-deserializer-not-passing-fork-inst branch May 12, 2022 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fork_inst not being passed along to element deserializer in default list deserializer
3 participants