-
Notifications
You must be signed in to change notification settings - Fork 20
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
Upgrade to new spago #35
Conversation
|
||
package: | ||
name: spec-discovery | ||
dependencies: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the dependencies have to be explicitly versioned? I see the weren't versioned in the old spago config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they weren't, but I think it's a good practice to specify version numbers nevertheless.
In Haskell you have things like: https://hackage.haskell.org/package/cabal-constraints which pin the versions exactly.
If spec releases version 8.x.x, someone will soon discover that purescript-spec-discovery
won't compile because of dependency errors and would try to fix it to work with newer version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the whole point of package sets to avoid these things?
For example, if there is a new version of spec that makes discovery not build anymore, then discovery won't be included in the next package set. And anybody who needs it can just stay with the previous package set version, not upgrade to the new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe you're right, I'll fix these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah no sorry, I forgot that spago publish
requires these things to be added. When I didn't specify the versions, it complained that it can't publish such a package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, good for now. I'll look into this some more later.
workspace: | ||
packageSet: | ||
registry: 50.7.0 | ||
extraPackages: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be omitted, right?
- spec: ">=7.6.0 <8.0.0" | ||
test: | ||
main: Test.Main | ||
dependencies: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this I guess?
@CGenie I think you need to update the GHA config too. See how GHA is failing? |
I guess we need nodejs at least 16, it's 14 in tests still. https://en.wikipedia.org/wiki/Node.js#Releases I guess version 22 is a safe bet now? |
Still failing |
Yeah because I forgot about bower. |
I don't understand github. I wanted to set up my own runner for this but I don't know how. |
Thanks! |
This PR contains an upgrade of this package to the new spago with
spago.yaml
replacingspago.dhall
,packages.dhall
,bower.json
.