Skip to content

Conversation

@Mashimiao
Copy link

Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com

@Mashimiao
Copy link
Author

ping @opencontainers/runtime-tools-maintainers

**--template**=PATH
Override the default template with your own.
Additional options will only adjust the relevant portions of your template.
whether template is valid or not will not be checked, this may lead to finally generated config file can't work directly. Currently, users should guarantee template is valid themselves.
Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean by can't work directly?

Copy link
Author

Choose a reason for hiding this comment

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

For example, as we don't have template validation now, if there are duplicated namespace entries in user's template, the final config file will also have duplicated namespace. Then the config file is invalid and can't be used directly.
We need to solve such problems, but not easy, should reform and reuse validate fucntions, currently adding warning will be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Mashimiao Thanks! Maybe word this as
"Templates are not validated for correctness, so the user should ensure that they are correct."

Copy link
Author

Choose a reason for hiding this comment

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

fine, updated

Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
@Mashimiao
Copy link
Author

@opencontainers/runtime-tools-maintainers PTAL

@liangchenye
Copy link
Member

@Mashimiao I think we should add 'template validation' .

@mrunalp
Copy link
Contributor

mrunalp commented Jun 28, 2017

We can add that in a follow-on I think. LGTM.

@Mashimiao
Copy link
Author

@liangchenye If we want to add 'template validation', we need to make some modifications on Generation and validaiton. I think we can't finish them in short period, so I submitted this PR to give a warn to users.
Agree with @mrunalp, we can add it in another PR in the future.

@liangchenye
Copy link
Member

OK, I add an issue to keep track of this. #403

@liangchenye
Copy link
Member

liangchenye commented Jun 29, 2017

LGTM

Approved with PullApprove

1 similar comment
@hqhq
Copy link
Contributor

hqhq commented Jun 29, 2017

LGTM

Approved with PullApprove

@hqhq hqhq merged commit 175af97 into opencontainers:master Jun 29, 2017
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.

4 participants