Skip to content
This repository has been archived by the owner on Nov 21, 2018. It is now read-only.

WIP - Adds build process (Gulp.js-based) #119

Closed
wants to merge 40 commits into from
Closed

Conversation

snostorm
Copy link
Contributor

@snostorm snostorm commented Feb 5, 2015

For some (easier) transparency in to the progress of this branch I've opened a PR. As of today I assume it is not in a merge-ready state but I did want to get easier access for us to review the ongoing diff and moving towards a merge sooner-than-later.

I don't think we need to get everything perfect early on, just solid/simple enough with a reasonable enough pattern we can build some additional content off of.

@snostorm snostorm added pr and removed pr labels Feb 5, 2015
@therebelrobot
Copy link
Contributor

Agreed that this is not yet ready, but that it provides better transparency on the work we're doing. :)

@snostorm
Copy link
Contributor Author

snostorm commented Feb 8, 2015

What are the current blockers? Maybe we can edit the PR description at the top to include a checklist.

@therebelrobot
Copy link
Contributor

The only blocker on this was time. I'm dedicating a good 6-8 hours on this today, and this PR should be ready to merge.

@snostorm
Copy link
Contributor Author

snostorm commented Feb 8, 2015

Need any help today @therebelrobot ? Whether with review, testing or breaking off some sub tasks?

@therebelrobot
Copy link
Contributor

@snostorm, I'm almost there currently, the index is compiling fine, as the screenshot above shows. Working on breaking out the faq and es6 pages next, if you want to take one of those, convert the content into markdown and tweak the css, that'd be spectacular.

@therebelrobot
Copy link
Contributor

The only thing this doesn't cover is defaulting to /en/index.html on initial load. Do we want to put in a meta refresh tag in the root of public for this? Or do we want to put that off onto nginx to handle?

Yeah, I guess that may be more of a question for @mikeal or someone on build, doesn't look like it got answered above.

@therebelrobot
Copy link
Contributor

We should probably put a TRANSLATION.md into this PR to ensure that translation groups are following the procedure we need them to. I'll get that worked up right now.

Also added a TRANSLATION.md file to increase awareness of how to
contribute to translation
@therebelrobot
Copy link
Contributor

Ok. We should be good to go now. I added index.html, es6.html and faq.html with redirects into /en for now. If we want to use nginx later we can. Also, I added a TRANSLATION.md file to make sure localization groups know how to contribute to the translation of the site. I'm off to dinner for now, but I've got my phone on me, hit me up if something needs changing.

@therebelrobot
Copy link
Contributor

Ack, I forgot to squash the commits. I'll get that in.

@therebelrobot
Copy link
Contributor

Wow. I am royally mucking up this squash. Anyone want to take that? Everything should be ready to go.

@mikeal
Copy link
Contributor

mikeal commented Feb 8, 2015

I'm terrible at squashing :( @Fishrock123 is awesome at it though.

@Fishrock123
Copy link
Contributor

If i squash this, it will look like me just copying the directory overtop of the current head and commiting it XD

@Fishrock123
Copy link
Contributor

I guess I could try that octopus merge ben told me about..

@fengmk2 fengmk2 mentioned this pull request Feb 9, 2015
3 tasks
@fengmk2
Copy link

fengmk2 commented Feb 9, 2015

Should we ignore all auto build out *.html files?

@Fishrock123
Copy link
Contributor

Will do the squash and merge tomorrow.

@snostorm
Copy link
Contributor Author

snostorm commented Feb 9, 2015

@fengmk2 we're including generated .html files in public right now as our web server requires them for publishing purposes. Later we'll be ignoring this, yes.

@snostorm
Copy link
Contributor Author

snostorm commented Feb 9, 2015

As much as we want to rush this in for speed of facilitating our i18n teams, I'd rather hold on this. After further review/testing the main templates task I noticed it was running especially slow for me. Since then I've already had some success with speed gains in the refactoring. I've been moving this file slightly closer to a typical gulp pipeline flow to squeeze out some performance and reduce some file reads/repeated Array parsing.

(The slowness was especially noticeable post watch-trigger while having to wait for the files to reload. On my typically zippy machine I could actually see the HTML appearing in files line-by-line.)

Anyway, I don't want to block this long. If i18n groups are excited to translate, let's get them forking off of the content directory in this branch for the next 24h or so then do a big PR + merge party after this goes live.

@mikeal
Copy link
Contributor

mikeal commented Feb 9, 2015

Don't block this on build performance, having something that is kinda slow is better than having nothing :)

@Fishrock123
Copy link
Contributor

Squashed cleanly, how will this interact with the nginx config? Do I need to PR a redirect to /en/ there for now?

@Fishrock123
Copy link
Contributor

Oh, that landed in 2cdc6c4. Fixing up and landing this.

@fengmk2
Copy link

fengmk2 commented Feb 9, 2015

💯

@Fishrock123
Copy link
Contributor

@snostorm the css tasks run 22 times slower than the template tasks for me. The template build takes half a second, compared to 11 seconds.

(Also I dunno why the css build is quite that slow..)

@Fishrock123
Copy link
Contributor

(unless the templates task end isn't actually at the end of the templates build..)

timaschew pushed a commit to timaschew/website that referenced this pull request Feb 13, 2015
Uses Markdown and html-template for content templating.

PR-URL: nodejs#119
Closes nodejs#119
Closes nodejs#22
See also nodejs#33
See also nodejs#125
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants