Samuel Rivas samuelrivas

  • Stockholm, Sweden
  • Joined on
@samuelrivas
samuelrivas merged pull request afiniate/afin#3
@samuelrivas
Add find and slurp functions
5 commits with 52 additions and 28 deletions
samuelrivas commented on pull request afiniate/vrt#14
@samuelrivas

I have reviewed that one. It has a minor comment, but is either ready for merging if you drop the comment or almost ready changing one line

samuelrivas commented on pull request afiniate/vrt#14
@samuelrivas

I am not sure I am suggesting "sticking the format commands into main function". Do you mean calling format_cmds directly from write_opam? If that …

samuelrivas commented on pull request afiniate/vrt#14
@samuelrivas

Shouldn't this use the function you created in afin? https://github.com/afiniate/afin/pull/2/files

samuelrivas commented on pull request afiniate/vrt#14
@samuelrivas

Same thing as commented in line 18 of opam_make_opam

samuelrivas commented on pull request afiniate/vrt#14
@samuelrivas

There is still duplication in these create_XXX_cmds they are all the of the form let create_build_cmds = f(x) where f(x) is something like format_…

samuelrivas commented on pull request afiniate/aws_async#4
@samuelrivas

Looks good to me

@samuelrivas
samuelrivas merged pull request afiniate/ddbi#2
@samuelrivas
Update ddbi to use the current vrt
2 commits with 212 additions and 128 deletions
samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

There are a few comments that have no answer, I bumped some of them. You'll also need to merge the PR in afin (https://github.com/afiniate/afin/pul…

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

I guess you refer to https://github.com/afiniate/afin/pull/2/files ? Call that from here then? It should be next to ready for being merged

samuelrivas commented on pull request afiniate/afin#2
@samuelrivas

you could use >>| here I think. But not a biggie

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Also pending

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Did you forget about this one or decided against it?

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

I'll check in the afternoon

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Good point, I keep thinking the haskell way :(

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Yes

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

I think Option.value should make this function look nicer

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

You can abstract this apply-string-transformation-if-not-None patter into a function and get rid of all those matches

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

I think you can join these create_XXX_cmds in a single function

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

This function could be in our generic library

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Isn't the fold just String.concat?

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

Couldn't these three functions be just one? The only thing that changes is the word after "make ..."

samuelrivas commented on pull request afiniate/vrt#13
@samuelrivas

I wonder if it is safer to write fun () to make sure we don't miss any (very unlikely) changes in Writer.save

samuelrivas commented on pull request afiniate/ouija#3
@samuelrivas

Ok, that is a problem of lacking documentation :) The point of Basic vs Async is not really whether the fold is sequential or parallel (they are bo…

samuelrivas commented on pull request afiniate/ouija#3
@samuelrivas

Do you need to use async for these tests? Since the previous version was using Ounit I kind of expect Sentinel.Basic to be enough

samuelrivas opened pull request afiniate/vrt#12
@samuelrivas
Fix dot merlin and tests
3 commits with 3 additions and 4 deletions
@samuelrivas