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

Removes empty i18n articles #246 #247

Merged
merged 3 commits into from
Mar 4, 2015
Merged

Conversation

snostorm
Copy link
Contributor

For #246


```sh
iojs -p process.versions.v8
```

This comment was marked as off-topic.

This comment was marked as off-topic.

@Fishrock123
Copy link
Contributor

There's a not-yet-translated /de/ page in there, i think it should stay? cc @fhemberger

lgtm otherwise

@snostorm
Copy link
Contributor Author

The de article should be removed. The point is that any articles not translated just aren't included. Long term this will be necessary anyway as the number of pages continues to grow.

Since we're not yet linking to the i18n sections I'm also fine with having the "broken link" from the other two pages. I'll deal with falling back to the English articles automatically when we pseudo-404.

@fhemberger
Copy link
Contributor

We already have a PR pending for the ES6 page translation #229.

@snostorm
Copy link
Contributor Author

#229 is blocked by merge conflicts though. When #229 is merge-ready and lands @fhemberger, the file will be returned to the de directory for rendering within the site.

The issue here is either we remove all extra English articles or none at all (as an ongoing policy.) Especially important as we start to add new pages soon

@fhemberger
Copy link
Contributor

Ok, than I'm fine with the removal.

As a side note: This would probably also help to configure a fallback language. In this case, de/ES6.html does not exist and would return a 404, so nginx could try en/ES6.html first, before returning a 404 (at least this should be possible on Apache, so I guess a similar setup is possible for nginx as well).

@bnb
Copy link

bnb commented Feb 27, 2015

I agree with the remove all English articles from localized sites plan. However, should this be an issue that's +1ed by a majority (or whatever the minimum % of members established by the consensus seeking model is) of @iojs/website ? It seems like kind of a big decision in the long run, as it defines a strategy the website will be adopting.

@snostorm
Copy link
Contributor Author

Yup. I just setup the Issue/PR so we can land it if approved. With a WG meeting on Monday I'm willing to see this wait a few days -- although it is also an easy enough PR to reverse.

The risks to merging right now are fairly small though as this only affects content nobody is actually consuming. (The fallback case for the iojs/iojs-de repo is the single exception, although I'm not sure if anybody actually is linking to the de content yet. No links from their README, Twitter, etc. and our own site doesn't yet link to the i18n.)

@Fishrock123
Copy link
Contributor

I'm fine with removing un-used language directories, but if we remove individual pages we'll start having to adjust links all over the place so that they go to actual pages...

@snostorm
Copy link
Contributor Author

Well, as soon as we add more pages, we're going to hit that issue regardless @Fishrock123. Probably best to figure out a technical solution for that now, before we link up the i18n folders for public consumption.

@snostorm
Copy link
Contributor Author

Note this was updated to keep the @iojs/iojs-ru files now that they've landed in master.

@snostorm
Copy link
Contributor Author

This branch now contains a "fallback" es6 file for the de localization. (Redirects to English)

@snostorm
Copy link
Contributor Author

snostorm commented Mar 3, 2015

Approved the removal of empty articles in the WG meeting. I'll hold off merging for a little bit, pending some process improvement tinkering.

@snostorm snostorm force-pushed the 246_removes_empty_i18n_articles branch from cb0cfe8 to 00c77ec Compare March 4, 2015 15:18
@snostorm
Copy link
Contributor Author

snostorm commented Mar 4, 2015

Cleaned up this PR (some groups since added content) and merging.

snostorm added a commit that referenced this pull request Mar 4, 2015
@snostorm snostorm merged commit 3bfdd3c into master Mar 4, 2015
@snostorm snostorm deleted the 246_removes_empty_i18n_articles branch March 13, 2015 20:33
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.

4 participants