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

Lint templates #250

Merged
merged 22 commits into from
Oct 30, 2019
Merged

Lint templates #250

merged 22 commits into from
Oct 30, 2019

Conversation

mikegeyser
Copy link
Contributor

This PR includes prettier to do html reformatting and linting, so that we can catch errors in the markup at generation-time (both visually and programmatically). I've had to change some of the content files, so that the errors in them didn't break parsing.

I'm not sure if we want this at such a late stage, and I understand if this gets rejected, but I think that it will help catch and action errors sooner. (This change is built off of #232).

@mikegeyser mikegeyser requested a review from rviscomi October 29, 2019 06:37
@mikegeyser mikegeyser added the development Building the Almanac tech stack label Oct 29, 2019
@mikegeyser mikegeyser added this to the SHIP IT! milestone Oct 29, 2019
@rviscomi
Copy link
Member

This also includes TOC changes. Should the destination branch be TOC as opposed to master?

@mikegeyser
Copy link
Contributor Author

@rviscomi I can change the target, I did mention it in the PR message. My rationale was that I didn't want to delay #232 on the chance that we didn't want this change.

mikegeyser and others added 2 commits October 29, 2019 09:22
@rviscomi
Copy link
Member

I do think it's a good change! Let's make it work.

And sorry I accidentally pushed a commit to this branch, but it's actually semi-relevant so let's include it. I'm just rerunning the generation script and saving the markup changes.

@mikegeyser
Copy link
Contributor Author

I've merged #232 into this again, and regenerated the templates to check the output. I think it's working alright now?

@rviscomi
Copy link
Member

More merge conflicts 😭

@mikegeyser
Copy link
Contributor Author

Fixed conflicts. Really sorry for the churn around these two PRs. :(

Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mikegeyser! This looks great 👍

@rviscomi rviscomi merged commit 8d6fea5 into master Oct 30, 2019
@rviscomi rviscomi deleted the lint-templates branch October 30, 2019 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants