Skip to content

Conversation

arrowd
Copy link
Contributor

@arrowd arrowd commented Aug 17, 2018

Due to #3515 I wasn't able to test if Stack builds with this change, but changes made for bindings-uname do work.

@ghost
Copy link

ghost commented Aug 17, 2018

@mgsloan can you take a look at this PR ?

@mgsloan
Copy link
Contributor

mgsloan commented Aug 18, 2018

Looks like a reasonable change to me. I think it would be better to fix the bindings-uname package and update the one used by stack, though.

@arrowd
Copy link
Contributor Author

arrowd commented Aug 22, 2018

I've never worked with Hackage. How can I update bindings-uname there?

@dbaynard
Copy link
Contributor

dbaynard commented Aug 22, 2018

It hasn't been updated since 2009, and there's no website, though the git repo still works. Ping the maintainer an email?

Edit: relevant haskell-infra/hackage-trustees#79

@arrowd
Copy link
Contributor Author

arrowd commented Aug 22, 2018

Ping the maintainer an email?

Done that 8 days ago.

@dbaynard
Copy link
Contributor

Yeah, I think this package needs a new maintainer. If you're willing to do it, there's a process on the hackage site.

The upshot of that is, I think we should merge the PR (subject to code review) and then when there's a new maintainer, migrate to the new code.

Which were the changes you made from the original bindings-uname?

@arrowd
Copy link
Contributor Author

arrowd commented Aug 22, 2018

Which were the changes you made from the original bindings-uname?

Here is the patch: http://arrowd.name/uname.patch

@dbaynard
Copy link
Contributor

Thanks. I'd ideally like to see that as two separate commits.

Due to #3515 I wasn't able to test if Stack builds with this change

Is this because there's no binary version of stack that works and so you can't bootstrap to test the changes? We should be getting a static build of stack again, soon — would that be helpful on that front?

It might be good to prioritize the fix to #3515 at the same time so it can be tested. That issue mentions the fix by @Blaisorblade for OpenBSD in 3cd92c8. Would it be possible to do a similar thing here?

@arrowd
Copy link
Contributor Author

arrowd commented Aug 22, 2018

It has been tested by CI, it's all green. Doesn't that count?

@dbaynard
Copy link
Contributor

It does count, although the integration tests aren't run by CI. I've asked maintainers to comment, but I think this will be OK to merge.

@borsboom
Copy link
Contributor

I think it's OK to merge.

@arrowd
Copy link
Contributor Author

arrowd commented Mar 13, 2019

I just found out that this PR also didn't make it into stable, so on this branch sysRelease still return garbage :<

I hope, this will be included in the next Stack release.

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.

4 participants