-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect time measurements in unpublish workflow prevents unpublishing #652
Comments
I think there's value in preserving timestamps of old packages. We can take care of this issue by:
|
Good point on the second note there, and I agree with you we ought to preserve time stamps for old packages |
Would this just amount to distinguishing between when this |
I think you're right that it's basically choosing between two scenarios: one in which this is an import of a recent package initiated by a user's action, and one in which this is an automatic import of a legacy package. In fact, we already have a type for the distinction between the two: registry-dev/app/src/App/API.purs Lines 98 to 101 in 282a4f0
The easiest thing is probably to move this data type into registry-dev/app/src/App/Effect/Source.purs Lines 44 to 45 in 282a4f0
|
(The comment on the |
As seen in:
purescript/registry#346 (comment)
...a package published just minutes prior cannot be unpublished because, and I quote:
This error arises here:
registry-dev/app/src/App/API.purs
Lines 243 to 256 in 3166e09
But the real implementation is deeper, in the registry library itself:
registry-dev/lib/src/Operation/Validation.purs
Lines 133 to 158 in 3166e09
We're comparing the current time in the UTC time zone to the published time associated with the package, which is calculated here:
registry-dev/app/src/App/Effect/Source.purs
Lines 94 to 108 in 3166e09
The problem in this specific case:
The reason why we use the ref time as the publish time is to support the legacy importer, in which we need to associate old publish times so the registry doesn't say everything was published in August 2023. But we probably should be using the current time in the publish pipeline instead.
The text was updated successfully, but these errors were encountered: