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

Use custom YAML subclass to be compatible with ruamel_yaml>=0.17 #543

Merged
merged 1 commit into from Jul 4, 2021

Conversation

mweinelt
Copy link
Contributor

@mweinelt mweinelt commented Jun 20, 2021

Fixes a DeprecationWarning for ruamel_yaml>=0.17

/build/source/whipper/test/test_result_logger.py:167: PendingDeprecationWarning:
dump will be removed, use

  yaml=YAML(typ='unsafe', pure=True)
  yaml.dump(...)

instead
  ruamel.yaml.dump(
/nix/store/lf2qpwvjp3gxgrjkcvcw18d3zlrxsif8-python3.8-ruamel.yaml-0.17.9/lib/python3.8/site-packages/ruamel/yaml/main.py:1364: PendingDeprecationWarning:
dump_all will be removed, use

  yaml=YAML(typ='unsafe', pure=True)
  yaml.dump_all(...)

instead
  return dump_all(
F
======================================================================
FAIL: testLogger (whipper.test.test_result_logger.LoggerTestCase)
----------------------------------------------------------------------

ℹ️ Subclasses ruamel.yaml.YAML as whipper.common.yaml.YAML and moves common values for width and default_flow_style into whipper.common.yaml.YAML. RoundTripDumper, as I understand it, is the default dumper.

✔️ Tested with ruamel_yaml 0.16.13 and 0.17.9

❓ Not sure about the license header for whipper.common.yaml, since this is mostly copy/paste from the ruamel_yaml documentation (https://yaml.readthedocs.io/en/latest/example.html#output-of-dump-as-a-string)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💖 Thanks for opening your first pull request here! 💖

Copy link
Collaborator

@JoeLametta JoeLametta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! Don't know what to do about the license thing.

EDIT: This change also seems to fix a test failure which started manifesting with ruamel.yaml 0.17.5 (later versions too).

whipper/result/logger.py Outdated Show resolved Hide resolved
whipper/test/test_result_logger.py Outdated Show resolved Hide resolved
Signed-off-by: Martin Weinelt <hexa@darmstadt.ccc.de>
@JoeLametta JoeLametta merged commit 3c17c08 into whipper-team:develop Jul 4, 2021
@mweinelt mweinelt deleted the common-yaml-formatter branch July 4, 2021 15:43
@JoeLametta
Copy link
Collaborator

Merged, thanks!

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.

None yet

2 participants