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

Adds Validate Command #241

Closed
wants to merge 4 commits into from
Closed

Adds Validate Command #241

wants to merge 4 commits into from

Conversation

silentrob
Copy link
Collaborator

This will add a validate command the the CLI and does a preflight check on compile. A failing unrelated multihost test snuck into the branch so it is commented out.

Everything else is pretty much in order and self-explanatory, tests included.

@sintaxi
Copy link
Owner

sintaxi commented Feb 13, 2014

few things...

  • It looks as though this doubles our compile time. is this necessary? If so, is it really worth it?
  • is there a reason compile from the CLI validates but compile from the lib does not?
  • what dependency is leaking globals? lets fix it or remove the dependency.
  • this doesn't merge cleanly.

@sintaxi sintaxi closed this Feb 13, 2014
@silentrob
Copy link
Collaborator Author

The global leak was cause fixed by updating Mocha.
It should be updated now.

So the goal of this feature is to reduce the number of question and support around getting a failed compile. It boils down to people getting a half (or worse 99%) compiled app.

Perhaps we can come at the another way, bail harder in harp/terraform or have a cleanup on compile.
I figured the easiest approach was to implement a "dry-run" compile but I'm open to ideas on how best to solve this.

@sintaxi
Copy link
Owner

sintaxi commented Feb 13, 2014

I think the biggest problem is that when a compile gets to 90% completion and fails it looks as though it was successful because at first glance all the files appear to be there. I'm guessing that a cleanup will clear up the confusion in most cases.

I still think a validate function would be useful though but I don't see any reason to write to disk. It could simply render each file and omit the fs.writeFile step. This also means we wouldn't have to add temp as a dependency which is a bonus. Make sense?

@silentrob
Copy link
Collaborator Author

total agreement.

@sintaxi
Copy link
Owner

sintaxi commented Feb 13, 2014

cool. harp already has a built in cleanup helper so that should be pretty straight forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants