Closed ocharles closed 10 months ago
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Thanks @ocharles for the patch! Would you mind signing the CLA?
@blackgnezdo Done
This patch is great, but it would be even better if it bumped all references to ghc < 9.3
in the ghc-source-gen.cabal
and package.yaml
file.
thanks.
@stepcut I believe that's now done.
@stepcut I believe that's now done.
Closer! When nix builds this package it seems to look at the package.yaml
and regenerate the .cabal
from that -- so it would be sweet if you updated package.yaml
as well. I don't really understand what is going on here. All I know is that updating 9.3
to 9.5
in both files fixed it for me.
Thanks, I had missed that ghc-source-gen
used hpack
. I've updated package.yaml
and re-ran hpack .
.
Sadly it looks like our CI is broken. We can merge this, but until we have a working CI covering 9.4, I wouldn't feel comfortable making a release to hackage.
@jinwoo what do you think?
Thanks. Yeah, unfortunately we don't seem to have CI set up in this repo. It'd be great if someone can verify this PR with 9.4. Maybe you already did, @ocharles ?
It certainly builds with GHC 9.4, but I don't think I've actually ran the code that uses this dependency (I'm not responsible for that project). I'll try and see if I can do that and I'll report back.
I'm using ghc-source-gen
with https://github.com/mbj/stratosphere and would be happy to try generating on 9.4. In theory it should produce the same concrete syntax. This would validate this patch.
Can get to it later the week unless someone beats me to it.
@mbj Did you get a chance to try this? I unfortunately am not familiar with the bit of code at work that uses ghc-source-gen
, and the original author has now left. It might be easier if you try testing this, but I know it does at least build!
Sorry did not get around to do this, will try this weekend "OSS maintainer promise" ;) (I hope you get the sarcasm).
Any updates for this PR? I've tested this with proto-lens https://github.com/4eUeP/proto-lens/commit/073b75d991531bfbd59beb2d612367699f699d82, and it seems nothing broke.
Also, would you mind updating the small tool under the ghc-show-ast
directory to ghc9.4? @ocharles
@mbj Any chance of getting another OSS maintainer promise? :P
@ysangkok Happy OSS maintainer promise! I also get some pressure to release new stratosphere so this may fit.
Hey! We are also using this with proto-lens-protoc and test on CI. We have tested with GHC 9.4.5 and 9.4.6 and no issues have turned up so far.
@avdv Thanks for the update. Now that you confirmed this change works fine in your CI, I'll merge this now. Thanks!
@jinwoo Chances you can push a release to hackage?
OT: This PR also works also for my straosphere project, sorry I could not test it earlier: https://github.com/mbj/stratosphere/pull/225
Fixes #99.