purescript / pursuit

Website for hosting and searching PureScript API documentation
https://pursuit.purescript.org/
Other
170 stars 47 forks source link

Mark packages from purescript-deprecated as deprecated #400

Closed JulianLeviston closed 3 years ago

JulianLeviston commented 5 years ago

On the Pursuit page for purescript-generics, it's not possible to see that it's deprecated. I would have raised this on the purescript-generics repo, but it seems to be read only so I can't raise issues. Sorry if this isn't the right place to raise this, but I'm unsure if it's a problem with pursuit not being able to communicate deprecations, or the package maintainer simply hasn't marked it as deprecated within pursuit.

https://pursuit.purescript.org/packages/purescript-generics/4.0.0/docs/Data.Generic

Background: while creating a new project today where I was patterning off a previous project that used Data.Generic (ie purescript-generics), but upgrading purescript as well, I had a lot of installation conflict errors and I didn't realise the library was deprecated. I then subsequently had duplicate Data.Monoid imports, which felt strange given I had started from what I'd thought was a blank slate with just generics and thermite, then halogen.

hdgarrood commented 5 years ago

Thanks for the report. Pursuit has just recently gotten a feature enabling package deprecation -- https://github.com/purescript/pursuit/pull/391 -- so we should use that here.

hdgarrood commented 5 years ago

I'm going to unarchive the repo so that we can move this issue there.

hdgarrood commented 5 years ago

Actually I think we should probably track this on the pursuit repo, since there are a number of repos which should be marked as deprecated.

rolyp commented 4 years ago

This would be great. Googling “PureScript Data.Map” brings up the deprecated purescript-maps library as the top search result (for me, at any rate).

maxdeviant commented 3 years ago

@hdgarrood I would like to take a look at this.

These are the potential solutions that I have thought of so far:

  1. Add the pursuit-deprecated keyword to any deprecated packages and republish them
    • n.b., I'm not really a fan of this solution, as publishing new versions of deprecated packages seems like a rather large job/headache
  2. Add the pursuit-deprecated keyword to the metadata for the deprecated packages stored in Pursuit's database
    • Are there circumstances that would cause the metadata to be regenerated and thus have the deprecation keyword removed?
  3. Modify Pursuit to resolve the package's repository against GitHub and see if it resolves to the purescript-deprecated organization
    • This seems like it would add quite a bit of overhead and more calls to GitHub, as we wouldn't be able to tell if a package is deprecated or not until we actually make the request
  4. Maintain a list of deprecated packages in Pursuit and check packages against that to see if they should be marked as deprecated
    • This list could either be static or generated dynamically (e.g., pull the list of repositories in the purescript-deprecated org when Pursuit starts up)

Options 2, 3, and 4 would all most likely require removing/regenerating the cache entries for any affected packages.

Do any of these stand out to you as a preferred solution? Something else I haven't considered?

hdgarrood commented 3 years ago

The main thing is that deprecated packages only end up in purescript-deprecated if they originally lived in the purescript or purescript-contrib organizations. If any other package is deprecated, it won't be transferred to the purescript-deprecated org. I would prefer that package deprecation work the same way regardless of who originally created the package, and I also don't want to set a precedent of manually fiddling with the database files to handle package deprecation now that we have a sensible API for it (even if publishing a new version is a little bit of a faff). So for me, option 1 is the most attractive; I don't want to build in any assumption that packages are deprecated if and only if they live under the purescript-deprecated org.

As for option 2, there are potential circumstances which would cause the Pursuit database to need to be regenerated: the most likely one is a non-backwards-compatible change to the JSON format. These are rare, but they do happen occasionally, most often because of changes to the way types are represented inside the compiler (because some of the same types are used for producing the pursuit JSON upload; this is something I'd like to change eventually). We might find that there isn't a good way of preserving backwards compatibility with PolyKinds, which are coming in v0.14.0, for example, and that would require regenerating the Pursuit database. I've used this somewhat hacky script in the past for this.

maxdeviant commented 3 years ago

I would prefer that package deprecation work the same way regardless of who originally created the package

I'm in complete agreement on this.

As for option 2, there are potential circumstances which would cause the Pursuit database to need to be regenerated: the most likely one is a non-backwards-compatible change to the JSON format.

This is good to know.


I can go ahead and organize the effort to deprecate the packages living in purescript-deprecated. I'll probably open a new umbrella issue in this repo with a list of packages that need to be deprecated and then start working through it.

hdgarrood commented 3 years ago

Thanks! You might also run into https://github.com/purescript/pursuit/pull/391#issuecomment-504764605 so that’s worth bearing in mind I think. That might require code changes to Pursuit.

maxdeviant commented 3 years ago

Noted. Once some of these deprecated packages start making their way onto Pursuit I'll take a look to see what all needs to be done for them to be properly marked as deprecated.

@hdgarrood As a more general question regarding the cache, what do you think about purging a package from the cache when a new version is published? This would make new package versions behave the same as when a package is first published to Pursuit in that the cache would be generated upon first access.

maxdeviant commented 3 years ago

On second thought, we probably don't want to purge the entire package from the cache, as this would require regenerating the docs for every version. We would probably just want to consider purging things that could have changed from the cache (which I think would just be the index entry for the package).

maxdeviant commented 3 years ago

It looks like we might not need to make any changes to Pursuit.

purescript-strongcheck-laws (not one on the list, but one that depends on a purescript-deprecated package) was republished with a deprecated version and is showing up as deprecated on Pursuit:

Search Results

image

Package Page

image

hdgarrood commented 3 years ago

We would probably just want to consider purging things that could have changed from the cache (which I think would just be the index entry for the package).

I think in this case all pages across all versions of a package would need to be purged on package deprecation, since the "deprecated" badge also appears on every module docs page for a deprecated package, e.g. https://pursuit.purescript.org/packages/purescript-strongcheck-laws/2.0.0/docs/Test.StrongCheck.Laws

purescript-strongcheck-laws (not one on the list, but one that depends on a purescript-deprecated package) was republished with a deprecated version and is showing up as deprecated on Pursuit

Great, that is good news. I have a slight worry that this might be working just because the caching mechanism is not set up properly at all right now, though...

I would quite like to completely tear up the existing caching mechanism and replace it with something simpler. I think the fact that we only generate the cached files on demand, once they are requested, is the cause of most of the unnecessary complication. If we always generated or regenerated all the relevant files immediately on package upload, we could simplify the implementation of the Yesod app quite a bit - I think almost all inbound requests could even be handled entirely by nginx that way, and perhaps only the POST /packages and search requests would need to be proxied to the Yesod app.

maxdeviant commented 3 years ago

Great, that is good news. I have a slight worry that this might be working just because the caching mechanism is not set up properly at all right now, though...

I think you might be right here, as I checked earlier and the deprecation badge appeared on the package page but not on the search page. I didn't really dig much into it at the time, and when I went to reproduce later the badge was there.

It certainly sounds like the caching layer could be simpler, although it seems like that's not something we'll need to worry about for the moment (as much as it concerns deprecating these packages).

thomashoneyman commented 3 years ago

These packages now have the pursuit-deprecated keyword to indicate their deprecation. For example:

https://pursuit.purescript.org/packages/purescript-generics/4.0.1

JulianLeviston commented 3 years ago

Closing because it seems to be resolved. Thankyou! 😻