Nothing Special   »   [go: up one dir, main page]

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

DRAFT: yamlinclude initial commit #1038

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DRAFT: yamlinclude initial commit #1038

wants to merge 1 commit into from

Conversation

ravindk89
Copy link
Collaborator

This just has the basic YAMLINCLUDE logic.

I have to figure out how best to use this. The place it might make the most sense is the absurdly long common-mc-admin-config.rst, but before doing that I want to let #1028 complete.

So for now will let this sit and think about whether this actually adds value to our workflow.

@ravindk89 ravindk89 added the WIP Work In Progress (Review Only If Requested) label Oct 16, 2023

with open(path, "r") as file:
try:
yfile = yaml.safe_load(file)
Copy link
@i80and i80and Oct 16, 2023

Choose a reason for hiding this comment

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

Limit the try block to just this line

@@ -0,0 +1,96 @@
import os.path
Copy link

Choose a reason for hiding this comment

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

I would recommend running black on this file

#
# We parse that into a dictionary for lookup later as part of the substitution loop.
subs = {}
if ('subs') in self.options:
Copy link

Choose a reason for hiding this comment

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

No need for parenthesis -- makes it look at first glance like you might be creating a tuple

# We parse that into a dictionary for lookup later as part of the substitution loop.
subs = {}
if ('subs') in self.options:
for i in self.options.get('subs').splitlines():
Copy link

Choose a reason for hiding this comment

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

Just self.options["subs"] is fine. get() is unnecessary


if ('debug') in self.options: print ("Substitution dictionary is "); print(subs)

key = self.options.get('key')
Copy link

Choose a reason for hiding this comment

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

You probably want self.options["key"] unless you actually want to handle the case where key doesn't exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work In Progress (Review Only If Requested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants