Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add better logging to CLI #1065

Closed
10kc-zack opened this issue Mar 20, 2018 · 3 comments
Closed

Add better logging to CLI #1065

10kc-zack opened this issue Mar 20, 2018 · 3 comments
Labels
Milestone

Comments

@10kc-zack
Copy link

Issue

When running the new v4 CLI, I found that the messages being shown when using the CLI (or lack thereof) made it difficult to understand what what happening. For example:

This works as expected and creates an HTML version of my template:

mjml ./src/templates/my-template.mjml -o my-template.html

while this doesn't work:

mjml ./src/templates/my-template.mjml -o ./dist/my-template.html

When running both of these commands, neither one showed any sort of logging. After looking at #1014, I saw @iRyusa state that the CLI does not create the dist folder to avoid issues, but anyone else running this command would be unable to determine why both commands aren't working. I think it's important to follow the example set by large CLIs like create-react-app (CRA) that show progress of the CLI operations, or at the very least a meaningful error or success message. For example, CRA uses the following success and fail messages when trying to create a new project (which could be analogous to transpiling from MJML to HTML):

Success Case
screen shot 2018-03-20 at 5 28 36 pm

Fail Case
screen shot 2018-03-20 at 5 28 51 pm

In the scenario I described above for the MJML CLI, I think messages like this should be included (at the very least):

Success Case

Compiled MJML template to /path/to/my-template.html

Fail Case

Error! Failed to compile MJML template. Directory /path/to/dist does not exist.

MJML version:

v4.0.2

@iRyusa iRyusa added the CLI label Mar 21, 2018
@iRyusa
Copy link
Member

iRyusa commented Mar 21, 2018

Hi @10kc-zack

On this particular case, we've added a message in cli to warn you that output directory doesn't exist.
I'm not against a "verbose" mode/option for the cli, but default output should stick to the strict minimum, because some existing connectors in MJML ecosystem (ex: some ruby gems) interact directly with stdout.

@10kc-zack
Copy link
Author

Thanks for your reply @iRyusa. That makes perfect sense why you would want to keep logging to a minimum.

Is the message you mentioned adding in v4.0.2? I was not seeing a message when using the command mjml ./src/templates/my-template.mjml -o ./dist/my-template.html when ./dist did not exist, which prompted me to post the issue.

@iRyusa
Copy link
Member

iRyusa commented Mar 21, 2018

It might only works when output multiple files inside a non-existing directory 🤔

We'll check about that in the 4.1.0 version

@iRyusa iRyusa added this to the 4.1.0 milestone Mar 21, 2018
@ngarnier ngarnier added this to Soon to be released in MJML Roadmap Apr 24, 2018
@iRyusa iRyusa closed this as completed Apr 30, 2018
MJML Roadmap automation moved this from Soon to be released to Released Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
MJML Roadmap
  
Released
Development

No branches or pull requests

2 participants