search

dlang / dub

Package and build management system for D
MIT License
678 stars 227 forks source link

Use a typed PackageRecipe for packagesuppliers #2485

Open skoppe opened 2 years ago

skoppe commented 2 years ago

Instead of dealing with raw json, it is better to parse it at first sight.

Geod24 commented 2 years ago

Yes! I've been wanting this for a while, but the problem is that it's a breaking change for library users. Instead adding a final function that forwards to the implementation might do the trick ?

skoppe commented 2 years ago

but the problem is that it's a breaking change for library users.

Ah, you mean people who inherit from AbstractFallbackPackageSupplier.

Hmm yes, likely not many, but still.

Geod24 commented 2 years ago

Ah, you mean people who inherit from AbstractFallbackPackageSupplier.

Hmm yes, likely not many, but still.

Actually, changing the return type of fetchPackageRecipe means anyone implementing the PackageSupplier interface. Still, likely not many, but there aren't many users of Dub as a library. Adding a final function that does the job would help in the short term, and we can just rework this interface in a future major version.

Geod24 commented 2 years ago

@skoppe : If you can find a small change to make (e.g. the [] => null changes) and PR it separately, that would whitelist you for future CI runs of Github, otherwise you'll need someone to keep on approving it every time you update this PR.

skoppe commented 2 years ago

@Geod24 I don't quite understand how final is going to help, maybe I am missing something.

The way I see it is that I can't just change the PackageSupplier interface, since it is public, and people might have inherited from it.

This means we need to keep the Json fetchPackageRecipe(string packageId, Dependency dep, bool pre_release) function for a while, and deprecate it.

Additionally I can't just add a Nullable!PackageRecipe fetchPackageRecipe(string packageId, Dependency dep, bool pre_release) function, without providing a base implementation. Not to mention that its name conflicts with the deprecated one.

I think I need to add a fetchRecipe function with a base implementation that just calls the original fetchPackageRecipe, while deprecating the latter.

Using that I can implement both fetchRecipe and fetchPackageRecipe in the current package suppliers.

Geod24 commented 2 years ago

I think I need to add a [...]

Yes, that's what I was recommending. final because "function body only allowed in final functions in interface".