pants icon indicating copy to clipboard operation
pants copied to clipboard

Move all non-generate related resolve types to `pants.engine.resolves`.

Open kaos opened this issue 3 years ago • 4 comments

This is a generalization for the lockfile resolves data types and unions that are not specific for only generating lockfiles by moving them out of the pants.core.goals.generate_lockfiles module they become reusable for more use cases than merely lockfile creation.

=== There is NO code change in this PR only re-organizations. ;) ===

This is a prep PR for an upcoming feature to introspect and diff lock files. My goal is to make it easy to see what has been changed in a lock file after running ./pants generate-lockfiles. Design proposal issue tbw after I've run some feasibility tests.

PS. The main motivation for this refactor is that I want to add another union type to KnownUserResolveNames that is for loading a lock files data in a generic way, not generating it so felt wrong to add that to the generate_lockfiles goal module, and creating a new union type for it didn't feel right either as the current ones were kind of generically named already.

kaos avatar Sep 17 '22 23:09 kaos

Picked plugin-api change label, although I think this technically wasn't part of the plugin API, as it potentially may have been exposed to plugins.

kaos avatar Sep 17 '22 23:09 kaos

Hah, I just now realized that I may not have to touch any of the types here, merely introduce new types and then leveraging existing union members. So this PR is not a requirement, but could still be worth considering if it is seen as an improvement in term of implemented architecture otherwise we can simply close it.

kaos avatar Sep 18 '22 14:09 kaos

Thanks! I think it maybe makes sense to split out into a dedicated file, but that should not live in pants.engine because these mechanisms have nothing to do with the Rust engine and are not "foundational building blocks" for Pants itself. Instead pants.core.util_rules is the right folder IMO.

So, we could have core.util_rules.resolves vs core.util_rules.generate_lockfiles. An alternative is that I wanted to in the past rename generate_lockfiles to simply be lock, but didn't to avoid churn.

Wdyt?

Eric-Arellano avatar Sep 19 '22 21:09 Eric-Arellano

that should not live in pants.engine because these mechanisms have nothing to do with the Rust engine and are not "foundational building blocks" for Pants itself.

Gotcha. I was jumping back and forth trying to figure out the right place for it :P

My head started spinning after a while, trying to make sense of all the various types and unions involved in the whole lockfile feature. What I'm after I think is to have the other side of the lockfile story in a generic way. Now we have the generate lockfile side, but we're missing the consume (enumerate/read) lockfile side (generically, there's the ecosystem specific versions that consumes lockfiles, naturally).

I think it needs a little more design work, and then update this PR after that. I'll convert this to draft in the mean time.

kaos avatar Sep 20 '22 21:09 kaos

Found another way to solve what I was going for..

kaos avatar Oct 31 '22 00:10 kaos