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

"#@data.values" [no space after @] should error with a helpful hint #532

Open
cppforlife opened this issue Nov 2, 2021 · 4 comments
Open
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request error msg improvement

Comments

@cppforlife
Copy link
Contributor

cppforlife commented Nov 2, 2021

Describe the problem/challenge you have
users may accidentally forget to have a space in foo: #@data.values after @ and be confused why the data value is not showing up in the output.

(https://twitter.com/stylishandy/status/1455051198935957514?s=20)


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

@cppforlife cppforlife added enhancement This issue is a feature request carvel triage This issue has not yet been triaged for relevance error msg improvement labels Nov 2, 2021
@pivotaljohn
Copy link
Contributor

pivotaljohn commented Nov 3, 2021

I'm tempted to generalize this issue as, "Whenever there's an unrecognized annotation, ytt should error; in some set of common situations include a hint."

"Common Situations" could include:

  • #@load(...
  • #@data.values...
  • (in general) #@{module-name}.... (e.g. #@json.)
  • (in general) #@{starlark-builtin}... (e.g. #@if, #@for, ...)

Where the hint could note the needed space:

ytt: Error: Overlaying data values (in following order: overlay.yml):
  Document on line overlay.yml:7:
    Map item (key 'foo') on line overlay.yml:9:
      Unknown annotation '@data.values.replicas' (did you mean '@ data...`, with a space after the '@'?)

Thoughts?

@pivotaljohn pivotaljohn added discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel triage This issue has not yet been triaged for relevance labels Nov 3, 2021
@cppforlife
Copy link
Contributor Author

cppforlife commented Nov 4, 2021

i like where you going with it. this does make me think about backwards and forwards compatibility. do we get a benefit for being able to use unknown annotations today? (e.g. if some data values passed in i can use newer ytt functionality otherwise fall back? if we check annotations early enough this would become problematic...)

there is something to be said about possibility of doing short-term hinting and then potentially filling in more comprehensive changes.

@pivotaljohn
Copy link
Contributor

this does make me think about backwards and forwards compatibility.

Ditto. I'm thinking of putting a feature flag on it (as in the user-facing feature flag). It would be more ytt-ish if we'd be stricter by default and be able to relax with the flag.

Something like... idonno... --ignore-unknown-annotations 😁 .

@pivotaljohn
Copy link
Contributor

pivotaljohn commented Nov 5, 2021

This idea is an aspect of #114

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution enhancement This issue is a feature request error msg improvement
Projects
Status: To Triage
Development

No branches or pull requests

2 participants