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

tiltfile: improve the unmatched image warning message. Fixes https://github.com/tilt-dev/tilt/issues/3410 #3430

Merged
merged 1 commit into from Jun 10, 2020

Conversation

nicks
Copy link
Member

@nicks nicks commented Jun 9, 2020

Hello @jazzdan,

Please review the following commits I made in branch nicks/ch7422:

fc66199 (2020-06-09 12:28:48 -0400)
tiltfile: improve the unmatched image warning message. Fixes #3410

Code review reminders, by giving a LGTM you attest that:

  • Commits are adequately tested
  • Code is easy to understand and conforms to style guides
  • Incomplete code is marked with TODOs
  • Code is suitably instrumented with logging and metrics

Copy link
Contributor

@jazzdan jazzdan left a comment

Choose a reason for hiding this comment

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

Nice! I think this is definitely better.

// 1) They didn't include any Kubernetes or Docker Compose configs at all.
// 2) They included Kubernetes configs, but they're custom resources
// and Tilt can't infer the image.
// 3) They typo'd the image name, and need help finding the right name.
Copy link
Contributor

Choose a reason for hiding this comment

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

The one that @jgoulah seemed to be encountering is a slight variation on this, because of misconfiguration the YAML that should have consumed this was never included.

I don't think it's possible to account for this at this point in the code, I think this is more of a job for facets. Just wanted to note it here.

Copy link

Choose a reason for hiding this comment

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

its similar to a typo but it also pops up when helm values don't render the image tag or if tht tag isn't what tilt expects

Copy link
Member Author

Choose a reason for hiding this comment

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

ya, i added some info to this to the comment. I have ideas on tooling to help with this, though it's slightly different than the facets approach. stay tuned!

@nicks nicks merged commit 524d7a0 into master Jun 10, 2020
@nicks nicks deleted the nicks/ch7422 branch June 10, 2020 12:44
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.

Provide more context for "Image not used in any deploy config"
3 participants