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

#22 Adds compiled assets directory #26

Merged
merged 3 commits into from
Jan 20, 2015

Conversation

snostorm
Copy link
Contributor

Note: requires another PR to be merged, nodejs/build#31

I've temporarily left an index.html and faq.html, with redirects, in the event the two project deploys do not happen to sync up.

This should be merged prior to nodejs/build#31

@zeke
Copy link
Contributor

zeke commented Jan 12, 2015

I expected this PR to be predicated on the introduction a build step, but that's not the case. So what is the point?

@snostorm
Copy link
Contributor Author

@zeke I would argue it is easier to have the https://github.com/iojs/build using this structure well ahead of time to make any transition easier. That way an "adds static generator" type PR here could be merged fairly effortlessly without having to worry about syncing up the build project then.

In general, when I went to start a PR experimenting with options for build steps, it just felt awkward to have the public files on the root (given we're not using gh-pages.) IMHO it is nice to do small, atomic changes for changes coming down the pipeline.

I'd like to propose a simple static site generator workflow soon, this is just step 1.

@zeke
Copy link
Contributor

zeke commented Jan 12, 2015

Thanks for the explanation. That makes sense.

we're not using gh-pages

How and where is the site hosted? Is this documented somewhere? I assume there's some process sitting in front of the static website that redirects the /downloads dir to another location.. but I'm guessing.

@snostorm
Copy link
Contributor Author

@zeke checkout #18 -- it is managed by the build repo, specifically: https://github.com/iojs/build/tree/master/setup/www -- My nodejs/build#31 modifies the https://github.com/iojs/build project slightly to sync from this repo's dist folder vs the root.

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

I'm going to hold off on merging the change proposed in build until it's reconciled with #27 in some way, priority now is to get something up on the site for 1.0.0 and this would break those changes because they are in the top level directory.

@snostorm
Copy link
Contributor Author

@rvagg yup, no rush, let's get through the v1.0 commit/tweak marathon then I can rebase this PR around the "final" v1.0 site

@Fishrock123
Copy link
Contributor

Let's do this after 1.0.0

@snostorm
Copy link
Contributor Author

I rebased this off of #27 so it is showing a lot of noise from that branch (since #27 isn't merged yet and this PR targets master.) But, at least it is now ready-to-be-merged any time after #27 lands.

@Fishrock123
Copy link
Contributor

@snostorm I was actually thinking more along the lines of /public, personally. (also harpjs defaults to that if we use it)

@snostorm
Copy link
Contributor Author

While dist is pretty common too, I feel like the death blow to it here is that we also have a dist on the actual published website that would have a completely different meaning.

I'm okay with public other than I hope to make it really clear that the files in public would be automatically overwritten at any time (so people don't make PRs against it.) Using dist would have been more clear, but again, the name collision factor does bother me.

One last should out for ... build or publish(ed)?

@Fishrock123
Copy link
Contributor

Ideally public would not contain anything in-repo.

@snostorm
Copy link
Contributor Author

I just rebased this to clean up the diff log but want to decide on a name before actually changing the folder name of this and nodejs/build#31

  • dist (conflicts with the naming of iojs.org/dist)
  • build
  • public 👍 👍
  • publish(ed)
  • release 👍

Contributors, feel free to edit this task and sneak on a vote 👍, or comment a preference if not

@therebelrobot
Copy link
Contributor

added vote :)

@snostorm snostorm changed the title #22 Adds dist/ directory #22 Adds compiled assets directory Jan 15, 2015
snostorm added a commit to snostorm/build that referenced this pull request Jan 15, 2015
@snostorm
Copy link
Contributor Author

FYI both PRs (here and build) both currently work with public now vs dist. This can be merged when somebody -- @rvagg -- is ready to help sync the two up.

Note: a pre-deploy here is fine (say 5 minutes before build updates) thanks to the (temporary) root-directory {index|faq|es6}.html redirect files I put in to handle the transition.

@snostorm
Copy link
Contributor Author

FYI not sure if we care to debate the name more / accept more votes / etc. I am just keeping the PRs up-to-date.

@therebelrobot
Copy link
Contributor

👍

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

how about for an easy migration path you duplicate everything in ./ and ./public (dist, whatever) so the webhook keeps on working, then when I've told you I've updated my end you can go ahead and do whatever changes you want to make here. I'm weary about trying to synchronize the two.

@snostorm
Copy link
Contributor Author

@rvagg as a migration path I already setup a basic redirect rule on the project so if this hits earlier (which it should) the root director {index,faq,es6}.html files will redirect to the public directory.

At worst for like 5-10 minutes people hit the homepage and end up on a page called http://iojs.org/public/index.html (which would work based on the current server rules.)

But duplicating for a few day or so works for me too if we think it'll be smoother.

<!-- NOTE FOR FUTURE DEVS: this is a _very_ temporary file
while iojs.org shifts server settings. Please remove me. -->
<meta http-equiv="refresh" content="0;./public/es6.html">
<script>window.location.href='./public/es6.html';</script>

This comment was marked as off-topic.

@therebelrobot
Copy link
Contributor

@snostorm, this has some merge conflicts that need to be resolved. Also, do we want to add this to the agenda for monday, or is it good to merge once clean?

@snostorm
Copy link
Contributor Author

Ya, this will probably keep conflicting as we change the master. I can rebase again in hopes a merge is coming.

I'd still prefer the merge is approximately synced up the build team efforts to deploy the related change. At least within the same hour window. The redirect works but we wouldn't want a ton of users to experience it either.

Conflict resolved for the moment. It is merge-ready, pending a 👍 from build that they will merge the sibling PR.

@rvagg
Copy link
Member

rvagg commented Jan 16, 2015

@snostorm can we coordinate on IRC about this? hit me up when you want something to happen, there's just a bit too much going on for me to digest at the moment so I need some hand-holding here to get this to happen smoothly.

@snostorm
Copy link
Contributor Author

@rvagg one day, when our paths cross again on IRC, I'll rebase this and we can coordinate a push ;)

@rvagg
Copy link
Member

rvagg commented Jan 19, 2015

keep in mind I'm in Australia, so GMT+10 or thereabouts

@snostorm
Copy link
Contributor Author

FYI @rvagg and I are putting this live

snostorm added a commit that referenced this pull request Jan 20, 2015
@snostorm snostorm merged commit f748837 into nodejs:master Jan 20, 2015
rvagg added a commit to nodejs/build that referenced this pull request Jan 20, 2015
@snostorm snostorm removed the pending label Jan 20, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants