Skip to content

Update trypurescript deps to match purescript repo versions #223

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

Merged
merged 6 commits into from
May 29, 2021
Merged

Update trypurescript deps to match purescript repo versions #223

merged 6 commits into from
May 29, 2021

Conversation

JordanMartinez
Copy link
Contributor

No description provided.

filepath -any,
Glob -any,
scotty -any,
build-depends: base ==4.14.1.0,
Copy link
Collaborator

@hdgarrood hdgarrood May 28, 2021

Choose a reason for hiding this comment

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

Can we leave the bounds as -any here, please? All of these dependencies are already sufficiently nailed down by purescript itself. The only ones where I think we should keep the bounds are scotty and warp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meaning, if a dependency version was listed as -any, keep it as that? Since your comment here was made on the base line, I'm getting conflicting information in this comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah sorry, I think base can be left alone too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've force-pushed some changes to reset earlier work. What needs to change now?

stack.yaml Outdated
- github: purescript/purescript
commit: 7ecc42669c69682996f2196ba2eef6c4ca827348
commit: 0d570f6e3828fc1a93c7c20252bccb04b7d2b0f6
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we switch back to using a released version of purescript now please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. One downside of this is that purescript depends on purescript-ast, which is going to be merged into purescript-cst. So, we'll need to update this again to drop purescript-ast once v0.14.2 is released.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's fine, we'll need to update this file at that point anyway.

- I'm following what is shown in purescript.cabal, so I'm not 100% sure
this is entirely correct (e.g. removal of `network`)
- all except http-types, which isn't used in purescript
flags:
aeson-pretty:
lib-only: true
these:
assoc: false
quickcheck: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to keep this in? It lets us avoid depending on quickcheck.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like it's an invalid flag. If I run stack build with that included, I get this error:

$ stack build
Invalid flag specification:
- Package 'these' does not define the following flags (specified in stack.yaml):
  quickcheck
- Flags defined by package 'these':
  these:assoc

Note: PureScript's stack.yaml file doesn't have that flag either.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh right ok sorry, I didn’t realise it had been removed.

data-default -any,
directory -any,
filepath -any,
Glob -any,
scotty -any,
scotty ==0.12,
purescript,
purescript-cst,
containers -any,
http-types >= 0.8.5,
Copy link
Contributor Author

@JordanMartinez JordanMartinez May 28, 2021

Choose a reason for hiding this comment

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

@hdgarrood Before I merge this, should http-types be updated to -any or to the lts-17.6 package set's version, which is 0.12.3?

And is it correct that -any will use whichever dependencies work with purescript and the other versions dependencies? I'm not familiar with this syntax unfortunately.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about this a little more I think we might as well use -any for all of the dependencies here. This project is only ever going to be built by stack, and the stack.yaml file already pins everything down to a specific version, so additionally specifying versions here doesn’t really add anything.

It’s more important to provide accurate version bounds when you’re making a library which other people are depending on - this part of the cabal file does the same job as the dependencies part of a bower.json or package.json file in a JS library. But that’s not what we are doing here so it doesn’t really matter.

@JordanMartinez JordanMartinez merged commit 23bc82d into purescript:master May 29, 2021
@JordanMartinez JordanMartinez deleted the updateDepsTov0.14.1 branch May 29, 2021 00:16
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.

2 participants