Skip to content

tiltfile: allow specifying a tilt version constraint#3172

Merged
landism merged 6 commits into
masterfrom
matt/version_requirement
Apr 15, 2020
Merged

tiltfile: allow specifying a tilt version constraint#3172
landism merged 6 commits into
masterfrom
matt/version_requirement

Conversation

@landism

@landism landism commented Apr 2, 2020

Copy link
Copy Markdown
Member

fixes #3159

uses the constraint syntax described here

@landism
landism requested review from maiamcc and nicks April 2, 2020 18:15
Comment thread internal/cli/down.go
Comment thread internal/tiltfile/version/version.go Outdated
Comment thread internal/tiltfile/tiltfile_state.go Outdated
Comment thread internal/tiltfile/version/version.go Outdated
return nil, errors.Wrapf(err, "error parsing version constraint")
}
if !rng(ver) {
return nil, fmt.Errorf("you are running Tilt version %s, which doesn't match the version constraint specified in your Tiltfile: '%s'", e.tiltVersion, constraint)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It would be nice to include a helpful error messaging like linking to the website saying howto upgrade.

Also explaining which version they should be running.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It would be nice to include a helpful error messaging like linking to the website saying howto upgrade.

Good call. I'll need to make sure we a page appropriate to link to.

Also explaining which version they should be running.

Yeah, that would be nice, but I don't see a cheap way to do it with the information available, and it'd greatly increase the complexity of this feature to figure it out. We're currently logging the constraint. I'm inclined to wait to see how much use this gets / whether that's a problem before worrying about it. Also how this fits into #3125.

@drubin drubin Apr 7, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good call. I'll need to make sure we a page appropriate to link to.

https://docs.tilt.dev/upgrade.html this is what I used in my extension. tilt-dev/tilt-extensions#9

Yeah, that would be nice, but I don't see a cheap way to do it with the information available, and it'd greatly increase the complexity of this feature to figure it out. We're currently logging the constraint.

Ah, I couldn't see a log message for this, so I assumed it wasn't there.

Was just a suggestion I don't think you should complicate it too much.

@nicks

nicks commented Apr 10, 2020

Copy link
Copy Markdown
Contributor

is this PR waiting on anything?

@landism

landism commented Apr 13, 2020

Copy link
Copy Markdown
Member Author

is this PR waiting on anything?

I was going to add a docs page with instructions for installing a specific version and link to that in the error message, but we can add that later.

@landism
landism merged commit 54d416b into master Apr 15, 2020
@landism
landism deleted the matt/version_requirement branch April 15, 2020 15:47
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.

Tiltifle should be able to specify min/max Tilt version

3 participants