Add a --help command, more verbose console logs, make dir if not present, and load before apply automatically. #9
Conversation
…grations, add a --help command, and make logs a little more verbose
There was a problem hiding this comment.
This PR is doing too many different things at the moment.
If you want to get these changes in, it should be 3 PRs.
- The logger changes, which I have given feedback on. This could include adding the help command.
- The change for automating making the directory if it doesn't exist, which I've also given feedback on.
- Removing the need to call load directly. For that last one, the load command should be removed in favor of the function that handles loading being called by all the other commands except for the init command.
There was a problem hiding this comment.
I'm against the addition of this file. Rather than using a custom wrapper around console.log, I'd rather switch to using std/log from deno. On the page linked below, it has an example for module authors.
There was a problem hiding this comment.
If switching to use logger.info and logger.error, we shouldn't be adding any color or prefixes ourselves. I believe if someone wants to format their logs a certain way, they can do so with log.setup.
For example, I'd be okay with replacing console.log("Connecting to database") with logger.info("Connecting to database"). I'm also okay with general message improvements. But I don't want to add a bunch of prefixes for the various commands like this PR currently does.
There was a problem hiding this comment.
For the logger name, go with "@udibo/migrate".
| function createMigrationDirectoryIfNotExists( | ||
| migrationsDir: string | undefined, | ||
| ) { | ||
| throw new Error("Function not implemented."); | ||
| } |
There was a problem hiding this comment.
This function isn't needed. Deno's standard library includes one for ensuring a directory exists. Just use that.
https://deno.land/std@0.220.1/fs/ensure_dir.ts
| logger( | ||
| "error", | ||
| "Migration table already exists. Have you already initialized migrate?", | ||
| ); |
There was a problem hiding this comment.
I'd prefer errors not ask a question. I'd just have it say 'Already initialized'.
|
|
||
| logger( | ||
| "init", | ||
| "Database has been initialised with migrations table and migration timestamp trigger.", |
There was a problem hiding this comment.
I don't think we need to give this much detail. I think just saying something like "Created table for tracking migrations" would be fine.
| } else { | ||
| console.log("command not found"); | ||
| if (command == "--help") { | ||
| help(); | ||
| } else { | ||
| logger("error", "Command not found or missing argument."); | ||
| } |
There was a problem hiding this comment.
Change to else if for the help command and else for the log regarding command not found.
| logger( | ||
| "info", | ||
| "Migrate allows you to manage your postgres migrations via the CLI and files in your codebase", | ||
| ); | ||
| logger( | ||
| "init", | ||
| "init allows you to initialize your project and creates a migrations table in your database.", | ||
| ); | ||
| logger( | ||
| "load", | ||
| "load allows you to add migrations to your database but not run them", | ||
| ); | ||
| logger("apply", "apply loads and migrates any unmigrated migrations"); | ||
| logger("list", "list shows you all your migration files and their status"); | ||
| logger("status", "status gives you an overview of your migrations"); |
There was a problem hiding this comment.
If adding a help command. It should log one message saying "Commands:" Followed by other messages with 2 leading spaces that have the list of commands with the descriptions. Similar to what deno does. You can use padEnd to make all the commands have spacing after them that makes them all line up. No need to do coloring for this.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/padEnd
|
Thanks for the feedback. I'm going on holiday for a couple of weeks but will pick it up when I'm back. |
Hopefully this resolves #8 a little bit. I'm opening this as a starting point for some ideas for which I think would make it easier for someone to pick this up quicker.
More verbose console logs.
This introduces some more verbose console logs, with some instructions to help people pick up what to do next. It also adds a
--helpflag which can be used on the root, which explains a little more how to use this tool.For example, now when
initis used it gives you some steps on how to proceed.Make directory if it doesn't exist
Just a nice to have where if you specify a directory for the
migrationsDirand it doesn't exist, it generates it.Combining apply and load
This introduces the load function into apply, so that rather than having it as two steps it just does it all at once.