Skip to content

Conversation

bitschmidty
Copy link
Contributor

Changes:

  • on individual newsletter pages, show available translations, if they exist
  • filters on homepage for only english posts
  • filters on newsletter page for english posts
  • supports language additions
  • small fix to newsletter layout html

Notes:

  • translations will be located in _posts/[language-code]/newsletters/
  • only handles newsletter types for now
  • simple language codes (en, ja) for selecting translations. can consider flags or other visuals as well.

Testing:

@jnewbery
Copy link
Contributor

ACK 009fd1f. This seems fine for a low-key rollout of localized newsletters.

If we continue to get japanese and other language translations, we could:

  • add flags/graphics/drop-down menu to make this a bit more pleasing on the eye
  • add a localized https://bitcoinops.org/<code>/newsletters/ page so international readers have a place to find all newsletters translated to their language
  • add documentation/contrib scripts to make it easier for people to contribute translations.

Thanks for doing this @bitschmidty ! And thanks for handling everything with CG.

@bitschmidty
Copy link
Contributor Author

Rebased and pushed a commit which filters out non english posts from the existing feed.xml

We should also consider adding language specific feeds in the future.

@bitschmidty
Copy link
Contributor Author

Created #251 to track non blocking suggestions to localization in the future

@jnewbery
Copy link
Contributor

jnewbery commented Oct 30, 2019

Did you copy the base feed.xml from somewhere? (I would have guessed here: https://github.com/jekyll/jekyll-feed/blob/master/lib/jekyll-feed/feed.xml but it seems like there are some differences). If so, can you split add feed.xml template which filters for english only into two commits:

  • the first adds the unaltered feed.xml file, so reviewers can use it to generate the feed and verify that there are no differences from master.
  • the second makes your modifications to feed.xml

@bitschmidty
Copy link
Contributor Author

Split commits for the feed into two:

  1. template xml file from 0.10.0 of the jekyll-feed plugin (our current version)
  2. tweak to the template file for filtering on english language only

Copy link
Collaborator

@harding harding left a comment

Choose a reason for hiding this comment

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

Concept ACK. One bug (need to also filter /en/publications) and one style suggestion. Thanks, @bitschmidty!

Tested by builiding site with and without this PR as is, then diffing static HTML. Then doing that again with the suggested translation on top of this PR. Also browsed a local preview of the test site with the suggested translation. Reviewed code.

@jnewbery
Copy link
Contributor

ACK 560573f.

Thanks for splitting out the feed change into two commits. I've tested the following:

  • with just the first commit (add jekyll-feed v 0.10.0 feed template file), there are only whitespace changes
  • with the first commit and sample japanese/other language posts, those posts are included in the feed
  • with the first commit, sample japanese/other language posts and the second commit (filter xml feed to english language only), the feed is the same as master except for whitespace. All non-english posts are filtered out.

Once you've addressed @harding's feedback, this is ready for merge.

@bitschmidty
Copy link
Contributor Author

Pushed 2 commits to address feedback from @jnewbery and @harding . Thanks for the review feedback as well as outlining your testing approaches!

Copy link
Collaborator

@harding harding left a comment

Choose a reason for hiding this comment

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

Tested ACK ba22ddd Thanks!

@jnewbery jnewbery added newsletters Publishing/translating/editing newsletters internationalization Making the site appropriate for multiple localizations labels Oct 31, 2019
@jnewbery
Copy link
Contributor

ACK ba22ddd

@jnewbery jnewbery force-pushed the 2019-10-localization branch from ba22ddd to f0b0377 Compare October 31, 2019 17:09
@jnewbery
Copy link
Contributor

I've squashed the two fixup commits into the relevant commits. Merging now.

Thanks for delivering this, @bitschmidty !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internationalization Making the site appropriate for multiple localizations newsletters Publishing/translating/editing newsletters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants