Adding style checks to zapier build

[BrunoBernardino]

Fixes #103

Initially I tried tackling it by really calling zapier validate but that command runs a lot more things that don't belong in the build or push context.

After that I tried to not make duplicate calls or generate the definition file more than once, which required moving some build tasks around.

Also added an option to show any write file error with --debug.

I tried adding tests but since this is for the whole zapier build or zapier push, it's very difficult. I ended up just making a lot of tests with apps, locally.

Relevant Trello Card.

What this changes

• zapier build now errors if there are failing style checks
• zapier push now errors if there are failing style checks in the build part, instead of the upload part (sooner)

Screenshots

Failing push because of validation now, on the build level.

Failing build for a non-style-check-related failure.

Passing push

[xavdid]

Sweet, looks great! Glad you were able to streamline the process here. The one thing that jumped out at me is that you were repeating some of the code from validate anyway (such as callAPI('style-check'). I looked in the validate command and it seems like it's doing the same schema validation and style check that we're replicating in the build command now.

Could you speak a little bit to the other things that validate does that you don't want included during the build step? There's a lot of pieces to how CLI is run, so I want to make sure I understand how the methods differ from each other.

If not running validate directly is the best choice, it might be nice to combine the repeated code (generate app definition, api call, print errors) into a method that's shared between validate and build. That way we're at least confident that changes happen both places if there are major changes going forward.

That being said, I'm not convinced these are blockers, so no more code changes may be necessary.

[BrunoBernardino]

Great points! Thanks for looking into this. Let me see if I can address all of your comments:

1. The main thing to note is the validate and definition "core" commands are running from different locations (locally in zapier validate and from the tmp dir for the build in zapier build), so they're not the exact same.

2. The api call seemed too small to abstract into a separate function, but I'm happy to do it if you think it's worthwhile. The validate and definition are already the most abstracted they can be, I think.

3. The main "extra" things the zapier validate does is related to output (it outputs much more information on success and errors, which isn't consistent within the context of how zapier build, zapier upload, and zapier push behave). Running the command itself makes it harder to have both command outputs behave consistently. Also, I don't think we want to allow skipping style checks when building, but it's fine when validating.

4. I completely understand the concern with the possibility of the commands not being in sync, and that's why I tried calling zapier validate initially directly. Unless we build a specific zapier validate --silent that isn't really silent but only outputs something like OK/NOTOK, it's not possible to accomplish that. While I don't think the code's at a point where we need to consider that the best possibility (seems like the duplication isn't that great, all in all), I think we'll eventually get there in the future, if the complexity keeps growing.

I hope that makes sense? Let me know your thoughts.

[xavdid]

Yep, that all makes sense!

When I had first looked, I thought the function could be something like validateSchemaAndStyle that takes an app definition, does the api calls and processes them. Looking back now I'm not sure they're similar enough for it to be worth it.

I say ship it!