-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add engines.turbo Validation
#1361
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
Add engines.turbo Validation
#1361
Conversation
|
@ObliviousHarmony is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey @ObliviousHarmony! I think that this functionality is definitely something we should consider; we're looking into what operating as a global installation would look like and this is a good step along that path. Things that this should account for:
What do you think? |
|
Thanks for the feedback @nathanhammond!
My first thought was that
Do you think supporting both might be ambiguous? Centralizing these kinds of constraints in
Along the lines of the above, how would you feel about the following changes:
This would make it trivial to expand
This seems fair, however, I have a question first. Is there anything local to |
|
So, in terms of delegating to the project-local version, that might look something like this: Alternatively, it might not delegate to a project local version and just error. And how that interacts with a versioned daemon is also a fun problem to solve. These are future hypotheticals, we haven't decided what path we should to take. When this one comes around it'll be an RFC. We're definitely not going to be JavaScript-only forever. If you look at our own repository we have a Go project in here. We currently wire it up via Lines 5 to 13 in e828731
We have already begun the process of moving out of We also have to account for things like teachability and transferability of knowledge. Integrating in one way with the JavaScript ecosystem and another way with (everybody else) means that we are creating additional impedance for people learning. The |
We are also operating a multi-language monorepo, so, I appreciate anything that makes working with that use-case easier. I'm kind of curious how you plan to handle dependency management in those cases though. As an aside, I've never worked with Taking a step away from this pull request for a moment though, I do want to stress that having a single source of truth for project names, scripts, and dependencies is nice. One of the things that created confusion when we used Nx was that each project had a configuration file that contained duplicate content that needed to be maintained in both files. We didn't gain anything from this difference and in a few cases, people added scripts to the wrong file and created problems in our tooling. All the Nx commands did was call scripts in The integration with each project's
I agree with you. Decentralizing the configuration like this would only make it confusing for people. It's much better to keep Turborepo's configuration together. I'll go ahead and make some changes to this pull request accordingly. |
We are too! But we definitely have ideas. Right now the order of operations is
From an internal document: "People have been rebuilding make with a prettier interface for decades." The fundamental behavior of builds haven't changed since
It's actually good to see this portion of our hypothesis panning out. From another internal document: "One of the most important aspects of Turborepo is that it does less, and yet still provides value." We're well aware that the seamlessness of the integration is an important part of the value proposition and will be very careful to avoid screwing that up. You should expect everything that currently works to still work, with opt-in to more complexity and (presumably) more reward.
Interesting proposal you have there. 😜 As to the actual implementation of this PR: I have no real concerns with your proposed approach. We need to have a conversation amongst the team about the priority of being able to stand alone before landing, but we would have that conversation to unblock this PR. Aside: we should make an active decision about the other places we could choose to read this information from:
(I don't have an opinion at this time, just enumerating.) |
As a consumer, ideally, Turborepo would take the step of reading dependencies from relevant project files. Node projects from a The only additional feedback I would add here is that currently, defining implicit dependencies with a separate pipeline that is otherwise identical is a little strange.
Do you ever feel like modern software engineering is just us trying to rediscover the practices and processes followed by electrical and mechanical engineers 50 years ago?
👍🏼
I'll argue that's more your proposal than mine, obviously. 😄 You weren't very ambiguous referencing the
That's good to hear, I'll go ahead with that proposed approach then.
I thought about this initially but felt it inverted the relationship between Combining that enhancement request and this pull request, the end result is that you would only ever use |
|
@nathanhammond I've updated the pull request based on your feedback.
|
|
Adding this PR to our weekly internal team meeting agenda to unblock. |
|
Okay, here's the team feedback:
When I circle back around to that would it be okay for me to cherry-pick your commit into my PR? |
Sure, there's not much to cherry-pick though 😄 Thanks for getting back to me @nathanhammond. |
|
Hey! We've gotten global This now has to be implemented in Rust, so I'm going to close this PR. Look for a new PR soon! |
Description
When using a global install of
turbothere is no way to be sure it is compatible with the version ofturborequired by a given project. There seems to be a standard for tools to support validating against theenginesproperty in thepackage.jsonfile. This pull request adds that validation toturbocommands.Closes #1359.
Testing Instructions
"1.0.0"constraint topackage.jsonin theengines.turboproperty./turbow.shand note that it returns an errorpackage.jsonconstraint to"^1.0"./turbow.shand note that it displays the help outputpackage.jsonconstraint to"test"./turbow.shand note that it displays an error