-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
elm-land: Rewrite and migrate from elmPackages to pkgs/by-name #382369
Conversation
Should I add an alias from |
3036ec6
to
73fe88c
Compare
@zupo what does |
|
you can see that node is attached to the port from pid 7231 that will probably be the elm-land process. But chrome is stuck in CLOSE_WAIT which I believe suggests the server closed the connection but chrome did not. What is a bit sus is that server is on ipv4 while chrome is connecting via ipv6. But I'm not an expert on networking especially in macos - But my guess is that problem is not with the package. if you have non nixified version running locally you can compare what that does. It might be attaching to the port using ipv6 perhaps? I think you maybe can even fix this in your hostfile - maybe you're not binding localhost to ipv4 address. I would also definitely try accessing http://127.0.0.1:1234 directly. |
@turboMaCk: thank you for taking the time to help me. It's definitely my machine/browser. |
4cef084
to
915e238
Compare
Verified the latest commit works on macOS! |
@Kranzes is there anything missing? I see this is still ina Draft. |
Signed-off-by: Ilan Joselevich <[email protected]>
Honestly I'm OK not doing test for this case. But I'm not sure how exactly this regressed either so I don't even have an idea about what could make sense to test. Plus this package is now completely decoupled from most things (apart from elm compiler). |
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.
LGTM
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.
Diff LGTM.
I was dreading that 😱. In any case, having the test in a later PR is perfectly fine. Thanks again for fixing this! |
Supersedes: #382339
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.