Heldkat patch 1 #1128
Heldkat patch 1 #1128
Conversation
gardener-robot
added
needs/review
|
|
cli/gardener_ci/compliance.py
Outdated
| @@ -256,8 +256,8 @@ def resource_as_dict(component, resource, resource_id): | |||
| resource_as_dict(c,r,i) for c,r,i in removed_resource_version_ids | |||
| ]))) | |||
|
|
|||
| outfile_new = f'{parsed.outfile_prefix}-added.yaml' | |||
| outfile_removed = f'{parsed.outfile_prefix}-removed.yaml' | |||
| outfile_new = f'os.environ['SOURCE_PATH']+'/versionDiff/{parsed.outfile_prefix}-added.yaml' | |||
There was a problem hiding this comment.
that won't work (syntax-error). also I think it is not nice to hardcode assumptions about presence of SOURCE_PATH-env-var. as discussed out-of-bands, check whether passing --outfile-prefix via ARGV might solve what you are trying to achieve
There was a problem hiding this comment.
I adapted the file to set the outfile_prefix parameter to None, so that I can call it with a self-defined string. Would this work?
There was a problem hiding this comment.
@heldkat : well... in principal: yes. however, this will de-facto turn the (thus-far optional) parameter into a required one
There was a problem hiding this comment.
How about now? This would give me the option to define it in the defaults-file but does not require it in my understanding
cli/gardener_ci/compliance.py
Outdated
| @@ -68,7 +69,7 @@ def diff( | |||
| ocm_repo_urls: [str]=None, | |||
| cache_dir: str=_cfg.ctx.cache_dir, | |||
| defaults_file: str=None, | |||
| outfile_prefix: str='resource-diff' | |||
| outfile_prefix: str= 'resource-diff' | |||
There was a problem hiding this comment.
I think according to our linter-rules (and pep8), there should not be a whitespace after = here (also, whitespace should be used consistently at least within one codeblock)
There was a problem hiding this comment.
ah, perfect. I was wondering why it is not passing the linter test
cli/gardener_ci/compliance.py
Outdated
| ocm_repo_urls: list[str] | ||
| exclude_component_names: list[str] = None | ||
| exclude_component_resource_names: list[ComponentResourceNames] = None | ||
| resource_types: list[str] = None | ||
| name_template: str = None | ||
| name_template_expr: str = None | ||
| outfile_prefix: str = 'resource-diff' | ||
|
|
There was a problem hiding this comment.
I think this is one line too much (pep8 and our linter mandates exactly two lines - see linter output (linked to your pullrequest))
Changing output file paths in def_diff Update compliance.py Update compliance.py Update compliance.py Update compliance.py Update compliance.py Removing whitespace to adhere to linter rules Update compliance.py Update compliance.py Update compliance.py Update compliance.py
heldkat
force-pushed
the
heldkat-patch-1
branch
from
606fb2a to
880f1d1
Compare
gardener-robot
added
the
status/closed
Release note: