search

purescript / registry-dev

Development work related to the PureScript Registry
https://github.com/purescript/registry
97 stars 80 forks source link

Add removals to batch package set API #476

Closed thomashoneyman closed 2 years ago

thomashoneyman commented 2 years ago

Closes #438 by adding the last piece: support for package removals. To that end:

  1. I've modified our existing package sets processing code to work on a Map PackageName (Maybe Version) instead of always having package versions available. No version indicates that a package is being removed.
  2. I have added validation (with @colinwahl) for package removals. The rule: you can remove a package only so long as nothing in the package set depends on it, unless that package is also being removed.
  3. I've beefed up our error reporting so we can be clear about why exactly an addition, update, or removal did not go through.
  4. I have split the batch processing function, processBatch, into two separate functions: processBatchAtomic, which fails if the entire batch can't go through, and processBatchSequential, which attempts to find the maximal set of packages from the candidates that can go into the next set. The batch API should fail if the whole suggested batch doesn't work, so that you can manually fix it. No one is manually fixing the automatic package sets, so those should find as many packages as possible.

Also, I fixed a bug in which we were reusing some logic from the automatic package sets and only considering packages from the last 24 hours. There's no reason to do that in the batch API — someone is specifically asking for these packages to be considered, regardless of when they were published.

With this in place the batch API should be complete.

thomashoneyman commented 2 years ago

Voila! Here are the results of testing this out on the registry-preview repository.

Package Removal This payload should remove the slug package: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198535618

Result: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198541198 https://github.com/purescript/registry-preview/commit/39d25e3e278500ba91a6b15069c2af9ec1f30b57

Failed Multi-Package Removal This payload should fail to remove halogen-hooks and halogen-store, because halogen-hooks-extra also relies on halogen-hooks: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198556329

Note: I had to edit this one because it uncovered bugs, which are fixed by commits 425bd54 and f52a036.

Result: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198562308

Successful Multi-Package Removal Alright! Now that CI has told me I missed a package that also must be removed, I can remove it: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198562798

Result: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198566575 https://github.com/purescript/registry-preview/commit/6b48e957e7fb2dab81fa383e9b9065f947f4f0f5

Successful Multi-Package Addition And now, finally, I can add all these packages back into the package set: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198575138

Note: I found a typo in the close issue function, which I fixed in 5f8b100. Issues should be closed when a batch processes without error.

Result: https://github.com/purescript/registry-preview/issues/4#issuecomment-1198586326 https://github.com/purescript/registry-preview/commit/02b1e80aafabc49f63e1c37000838d6efc39c354

This one's a bonus: I accidentally swapped two package versions, but still got a nice error from it. https://github.com/purescript/registry-preview/issues/4#issuecomment-1198574498

f-f commented 2 years ago

Great work! :clap: