Skip to content
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

Use current time as published date #656

Conversation

pete-murphy
Copy link
Contributor

@pete-murphy pete-murphy commented Aug 19, 2023

Fixes #652

@pete-murphy pete-murphy force-pushed the pm/652/incorrect-time-measurements branch from 9aa8906 to 6da962e Compare August 22, 2023 01:21
@@ -237,7 +237,7 @@ deleteVersion arguments name version = do
Just (Left _) -> Log.error "Cannot reimport a version that was specifically unpublished"
Just (Right specificPackageMetadata) -> do
-- Obtains `newMetadata` via cache
API.publish API.Legacy
API.publish PackageSource'Legacy
Copy link
Member

Choose a reason for hiding this comment

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

We typically only use quotes in this codebase to represent types or functions that are a tweak on another; could we use something like “LegacyPackage” and “CurrentPackage” as constructors instead?

Copy link
Contributor Author

@pete-murphy pete-murphy Aug 22, 2023

Choose a reason for hiding this comment

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

Yeah, sorry was intentionally leaving this as "ugly"/unconventional to remind myself to comment about alternatives and ask for suggestions about the naming, since I'm not sure what to call the type/constructors here. I'll just paste what I had started typing up in another comment:

This was originally

data Source = Legacy | Current

but after moving to Prelude that would conflict with Source here

data Source a = Fetch FilePath Location String (Either String FetchedSource -> a)
and with Current constructor here
data GitStatus = Current | Behind Int | Ahead Int

Another alternative would be keeping the original constructor names but hide Current when importing in the CLI.Git module, or renaming the GitStatus constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But also happy rename things to

data PackageSource = LegacyPackage | CurrentPackage

if that's preferable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 706ea23

@pete-murphy pete-murphy marked this pull request as ready for review August 22, 2023 02:46
@pete-murphy pete-murphy changed the title [WIP] First pass at #652 Use current time as published date Aug 22, 2023
@thomashoneyman thomashoneyman merged commit 4346c82 into purescript:master Aug 25, 2023
15 checks passed
@thomashoneyman
Copy link
Member

Thank you @pete-murphy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect time measurements in unpublish workflow prevents unpublishing
2 participants