dhall-lang / dhall-to-cabal

Compile Dhall expressions to Cabal files
MIT License
100 stars 19 forks source link

Refactor Dhall.Extra to use utils for build records of dhall >= 1.13 #110

Closed jneira closed 6 years ago

jneira commented 6 years ago

I've chosen the easy path of keeping the existing type and functions and making it alias for the dhall ones. Let me know if you think that the best option would be remove them and replace for the new ones.

quasicomputational commented 6 years ago

Negative diffstats make me happy. Good spot that these are redundant now.

ocharles commented 6 years ago

Thanks, this looks good. I'm in favor of ripping the aliases out entirely, if you don't mind adding that.

jneira commented 6 years ago

@ocharles sure, done!

ocharles commented 6 years ago

Nice, thanks!

jneira commented 6 years ago

@ocharles beginner git user question: i've seen merges from master in the pr but later i've done a rebase and a forced push... not sure if i should have done it

ocharles commented 6 years ago

Ah, a force push will probably remove the master merges that i've done. But that's no problem, I can always do it again :) They were fairly automatic merges as there were no conflicts.

ocharles commented 6 years ago

@Gabriel439 Is Hydra OK? This PR is stuck on waiting for a status report, but http://hydra.dhall-lang.org/jobset/dhall-to-cabal/110 doesn't even show the latest commit from jneira.

Gabriella439 commented 6 years ago

I believe this is because there was a rebase. If the derivation is the same before and after the rebase (i.e. the contents of the branch haven't changed) then it confuses Hydra because it doesn't trigger a new build, even though there is a new revision

ocharles commented 6 years ago

I know there won't be a new build, but I'm surprised there isn't even an evaluation for the latest commit.

Gabriella439 commented 6 years ago

Hydra has a "feature" where if the source directory hasn't changed it doesn't trigger a new evaluation

ocharles commented 6 years ago

Huh, that one is new to me, didn't know about that. Ok, I'll turn off the mandatory status check temporarily to get this merged then.

jneira commented 6 years ago

oh, sorry for the inconveniences, in other projects rebase seems to be the usual way to integrate master changes

Gabriella439 commented 6 years ago

@jneira: GitHub supports a "Squash merge" feature which allows you to merge using a single commit without rebasing your pull request. Under the hood this is powered by git merge --squash, which is the equivalent rebase-free command-line workflow