Implement log redirection interface

[unclechu]

Relates to #34 (satisfies second part).

1. Adds alternative runMigrationA and runMigrationsA functions with additional argument of type Either Text Text -> IO () where Left is an error message and Right is an info message
2. By default sends error messages to stderr instead of stdout
3. Usage info in migrate executable is printed to stderr instead of stdout in case arguments are incorrect and executable exits with error exit code

This is the second attempt to implement the feature after #35

[MasseR]

All in all, I like these changes a lot more than the previous ones. My comments can be categorized into a few sections

• Nitpicking
• Autoformatter
• Thumbs ups

As a 3rd party, I don't see any obvious blockers on this. Maintainer will have the final say.

[MasseR]

I know I said runMigrationsA in the previous review, but this could probably have a more descriptive name? I'm terrible at naming things.

[unclechu]

@MasseR Ha, I have the same problem. I couldn’t come up with something good. I could name it as runMigrationsWithCustomLogWriteFn but I’m hesitating to add this.

[MasseR]

It looks like you ran some kind of autoformatter to the source? I don't mind and if were the maintainer I would pass this, but maybe be on the safe side and avoid autoformatters on unfamiliar codebases? Formatting can be a touchy subject for some.

[unclechu]

No, I didn’t use no formatter. All changes are manual. I realized almost all the lines in the code are limited to 80 chars. But only some aren’t. So I split those lines into few.

[MasseR]

I'm still hesitant on it. These kinds of extra changes are just extra noise for the reviewer/maintainer.

[unclechu]

I had to touch this line anyway.

[MasseR]

This is a visible change in behavior, I'm hesitant on this. As far as changes go, this is one of the more benign ones, but does change the behavior

postgresql-simple-migration --help | grep foo
postgresql-simple-migration --help | less
postgresql-simple-migration --help 2>&1 | grep foo

[unclechu]

1. None of the commands you provided as an example are related to this line in an any way
2. There’s no such argument --help, I added it to Implement abstract interface for log writes #35 but didn’t in this MR, the command will fail
3. This will print to stderr only in case there were some errors while applying migrations

If you refer to the usage of printUsage:

1. If you provide correct -h argument it will, as before, print usage info to stdout thus, if you use the program correctly, it will work as before.
2. If you provide incorrect arguments the program, as before, will fail and (this changed in this MR) will print usage info to stderr as it should. If you rely somewhere on failure scenario caused by using incorrect arguments then you’re doing something wrong and you’re using the program incorrectly. I think you should not pay any attention to copying bugs in order to keep behavior the same unless you build some platform emulation tool (e.g. WineHQ).
3. Someone may try to use --help argument (which doesn’t exist), it’s the most default one for getting usage info. But to whom I’m telling this, you just tried to do so in your example! And in Bash, if you don’t use set -eo pipefail, the grepping from your example would succeed (before this change) but this is so only because Bash sucks, it sweeps most of the errors under the carpet by default. It is still a bug. But just for this case I thought it was a good idea to add --help argument alongwith -h one as a part of the MR (which I did in Implement abstract interface for log writes #35 but you were against it, I still think it is a good idea).

[pjones]

What's the status of this PR? Does anyone need any help?

[unclechu]

@pjones We’re just waiting for maintainer’s feedback.

[unclechu]

If you ask me it’s ready to go.

[unclechu]

@ameingast Let’s ping the maintainer in case he missed or didn’t receive a notification.