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

File Sync format specification: maybe don't have user data in the key part #1180

Closed
thomas-riccardi opened this issue Oct 19, 2018 · 1 comment · Fixed by #1847
Closed

File Sync format specification: maybe don't have user data in the key part #1180

thomas-riccardi opened this issue Oct 19, 2018 · 1 comment · Fixed by #1847

Comments

@thomas-riccardi
Copy link

#1039 added a key: value map under sync in the skaffold format.
The key is a local glob pattern, the value is the destination directory.

Example:

    sync:
      '*.py': .

Coming from k8s it is unusual to have user data as keys; it surprised me.

Digging into k8s design doc, we have:
https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#lists-of-named-subobjects-preferred-over-maps
There are whole discussions linked from there, notably about

  • readability (mostly in case of subobjects though)
  • merging definitions, which may or may not be of concert for skaffold

It lists exceptions to this though: "pure maps [...] as opposed to sets of subobjects".

I suppose if we later want to enrich the format with subobjects it would make things harder with the current format.

I am not (yet) familiar with all the skaffold format, I don't know if there are other cases like that, I couln't find any.

Question

Shouldn't we forbit storing user data as keys?

Suggestions

Either go full sub-object like in k8s:

    sync:
    - localPattern: '*.py'
      destination: .

(better names are welcome)

Or maybe support lighter format with string:

    sync:
    - '*.py:.'

But I'm not sure it's a good idea: it doesn't seem that much readable...

@r2d4
Copy link
Contributor

r2d4 commented Oct 19, 2018

I think this is the right decision. I initially tried option 2 but rejected it for the same reason. Subobjects would also make #1166 easier to implement.

I’m all for subobjects, even at the expense of brevity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants