Implement abstract interface for log writes

[unclechu]

Closes #34

What it brings to the table:

1. Use MonadIO instead of monomorphic IO
2. Provide polymorphic migrationContextVerbose which, as before, can be just a Bool but also any custom type
3. Implement MigrationVerbosity that provides an abstract interface for log writes
4. Implement default instance of MigrationVerbosity for Bool
5. Error log messages are written to stderr instead of stdout
6. Adds additional options --help and --quiet for their shorthands -h and -q
7. Keep backward compatibility, except that if you explicitly annotated MigrationContext type (because new type variable has been added)

Generally it gives you an ability to redirect log messages somewhere else instead of just writing them directly to stdout.

[MasseR]

Opinions from a user of the library:

There seems to be quite a bit of destructive and unrelated changes.

• Modified the command line handling
• Modified the output channel from stdout to stderr (if I interpreted the changes correctly)
• Added new dependencies, some of which might be an overkill
• Made destructive changes to the signatures of existing functions.

How I envision implementing this issue so that it's as clean as possible:

Create new functions

runMigrationA :: (String -> IO ()) -> MIgrationContext -> IO (MigrationResult String)
runMigrationA logger context =
  -- Copy the implementation of the original 'runMigration' here, replacing the calls to putStrLn with the logger callback

runMigration :: MigrationContext -> IO (MigrationResult String)
runMigration = runMigrationA putStrLn

This way you would expose the more powerful version, but leave the interface and behavior of the original runMigration as is, accomodating the maintainers wish for signature changes. Also no changes to cabal packages, no changes to cli handling, no changes to stdout/stderr. If one wants to add MonadIO, MonadUnliftIO support, they could be added in a separate MR.

That means no destructive signature changes to existing public functions or data-types - only extensions.

[MasseR]

You have modified the signature of an existing function which is against the maintainers wishes:

That means no destructive signature changes to existing public functions or data-types - only extensions.

[unclechu]

Yes, and this is mentioned in the description. It’s not really that destructive. All the existing code is still compatible, except just in case you annotated MigrationContext type explicitly.

[unclechu]

@MasseR

There seems to be quite a bit of destructive and unrelated changes.

I’d like to hear what do you mean specifically by a “destructive change”?

Modified the command line handling

Since I was giving Main module pretty many changes (not really many changes but in sense of amount of diffed lines) I decided to also add a minor change there by adding --help and --quite equivalents for their shorthands. I’m okay with removing them from this MR. I’d create a separate MR with using a library for option parsing instead of this manual argument parsing that has a lot of tricky cases and questionable in general.

Modified the output channel from stdout to stderr (if I interpreted the changes correctly)

Partially correct. Not for everything. I changed output specifically for those messages which are representing errors.

Added new dependencies, some of which might be an overkill

Original wish from #34 was:

Use MonadIO instead of IO

In order to solve this I had to use unliftio which provides IO-agnostic implementation of finally.

Made destructive changes to the signatures of existing functions.

Yes, this mentioned in the description of this MR. If you didn’t explicitly annotate MigrationContext in your code all the types are still compatible.

(String -> IO ())

This signature is insufficient in my strong opinion. It doesn’t give you any information whether it is an error or just an info message. I would define it as (Either String String -> IO ()).

-- Copy the implementation of the original 'runMigration' here, replacing the calls to putStrLn with the logger callback

This would not work since the code intersects all over the place. I’d implement runMigrationA first (and probably some other alternative functions too) and then use it instead of runMigration as a particular use-case. This would also wipe redundant copies of the code from the table. Anyway, “copy the implementation” is seldom a good idea in general.

This way you would expose the more powerful version

I absolutely don’t see in what way this is “more powerful”. Could you be more specific? As I said before (String -> IO ()) is not sufficient. Apart from that it would keep redundant verbose flag in MigrationContext whilst you can shut it up with just const (), not a big deal anyway. My main point is that it’s not “more powerful” in an any way, maybe equivalent but not more powerful.

---

I’m closing this MR in favor of making a new one. I’ll reduce the amount of changes to:

1. Add another function(s) with a control of where the log messages are written to with this type signature: Either Text Text -> IO ()
2. Error log messages are written to stderr instead of stdout (I insist this must be done, if this is a “destructive” change in an any way then major version should be upped)

[unclechu]

@MasseR See new MR #36