Skip to content

Conversation

@JellyBellyDev
Copy link
Contributor

I add two new add methods to fetch package info from new composer 2 API.

I tried to add spec test but I'm not understand why not work.

@robbieaverill could you help me understand?
Thanks

@JellyBellyDev JellyBellyDev force-pushed the feat/composer2-metadata branch from 884e865 to f92072b Compare August 24, 2021 12:23
@robbieaverill
Copy link
Collaborator

Hey @JellyBellyDev, thank you for taking the time to make this pull request. I've had a quick look - do you think it's possible to change from Composer 1 to Composer 2 API results under the hood, while keeping the API surface of this package the same? If that's possible, that would be my suggested approach, even if it's only for the Versions data collection. It would be nice to avoid adding the new methods like getComposer2() etc.

@JellyBellyDev
Copy link
Contributor Author

Hi @robbieaverill the new version of composer api expose two new endpoint:

  • /p2/$vendor/$package.json (only tagged releases)
  • /p2/$vendor/$package~dev.json (tagged releases and branches)

If we want change under the hood the method getComposer() we should use /p2/$vendor/$package~dev.json but we must apply this logic, because the new api don't return the name of the version as key of the object.

If we do this, wouldn't it be fair to also provide a method that uses the lite version (/p2/$vendor/$package.json)?

@robbieaverill
Copy link
Collaborator

Hi @JellyBellyDev, apologies for the delay in responding to you. I personally don't think it's important for this library to expose the underlying Packagist API responses. The fact that we have one public method that does this already is okay. If you want to add another protected method to expose the lite version, go for it. Hopefully we can avoid exposing this as a public method though.

Your logic change seems reasonable to retain compatibility with the existing data structure we provide here.

@JellyBellyDev
Copy link
Contributor Author

Hi @robbieaverill, thanks for the reply, don't worry about the late! ;)
Forgive me, but it is not yet clear to me what you want me to do!
Could you explain better?
Thanks

@robbieaverill
Copy link
Collaborator

Hi @JellyBellyDev, apologies. My recommendation would be to change the getComposer() method to return the new information from Packagist and apply this logic, but not create a new method like getComposer2(). Let me know what you think 🙂

@robbieaverill
Copy link
Collaborator

Hey @JellyBellyDev, are you still keen to get this over the line? It'd be a great addition to get merged

@JellyBellyDev
Copy link
Contributor Author

@robbieaverill forgive me, I completely forgot about this issue. Complete ASAP! ;)

@JellyBellyDev JellyBellyDev force-pushed the feat/composer2-metadata branch from f92072b to 179847d Compare December 17, 2021 11:26
@JellyBellyDev
Copy link
Contributor Author

@robbieaverill PTAL

I override the method getComposer with new API /p2/%s~dev.json like you suggested and add new method getComposerLite for the lite version with only tagged releases!

public function getComposerLite(string $package): array
{
return $this->respond(sprintf($this->url('/p/%s.json'), $package));
return $this->respond(sprintf($this->url('/p2/%s.json'), $package));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just confirming that the change of the API we consume in this method was deliberate? I have no problem with changing the underlying API we consume from v1 to v2. Is the intention of adding getComposerLite() to allow access to the original v1 API endpoint, or to return the same results as getComposer() except without dev dependencies?

Copy link
Contributor Author

@JellyBellyDev JellyBellyDev Dec 21, 2021

Choose a reason for hiding this comment

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

Yes! I confirm!
I add getComposerLite() to return the same results of getComposer() except without dev dependencies as descripted here in the Getting package data section.

@robbieaverill
Copy link
Collaborator

Thanks!

@robbieaverill robbieaverill merged commit 8047bcd into KnpLabs:master Dec 30, 2021
@JellyBellyDev JellyBellyDev deleted the feat/composer2-metadata branch January 4, 2022 10:09
@JellyBellyDev
Copy link
Contributor Author

it was a pleasure! 🚀

*/
public function getComposer(string $package): array
{
return $this->respond(sprintf($this->url('/p2/%s~dev.json'), $package));
Copy link
Contributor

Choose a reason for hiding this comment

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

Be careful. The ~dev.json files is a not about getting all versions. It is about getting dev versions. None of the releases (be them stable releases or beta releases) are in that file. If you want to get all versions, you need to load both files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants