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

Issue 933: Add translators to narrative parsing #937

Conversation

ncallaway
Copy link

This adds translators to the narrative frontmatter parsing as described in #933.

Notes

  • I noticed there was a warning for authors as arrays not being implemented yet. I decided that the best path forward would be to implement authors as arrays or strings, then share that code with the translators parsing. The end result is that authors and translators have the same semantics, and can be given as either a single string, or an array.
  • While the issue doesn't specify it, sharing the same code means translators also has a translatorLinks key, which can be used to link to translator (a personal blog, mailto, twitter account, etc). translatorLinks are not necessary, and can be omitted for any/all translators.
  • This adds validation around the links.
    • If the authors attribute is a string, then authorLinks must be a string. If authors is an array, then authorLinks must be an array of the same size (null can be used for any element that doesn't have a link).
    • The same is true for translators and translatorLinks.
    • If the validation fails, the link is omitted and a warning message is displayed that explains specifically what failed the validation.
  • The only documentation I found on authors and authorLinks appeared to be in a tutorial. I didn't see reference documentation. Because I didn't want to bog down a tutorial with a more obscure attribute, I didn't add documentation for this. If there’s reference documentation anywhere point me at it and I can the documentation for these new attributes.

@jameshadfield jameshadfield temporarily deployed to auspice-issues-issue-93-iuswhj March 13, 2020 05:08 Inactive
@jameshadfield jameshadfield merged commit 44842ae into nextstrain:master Mar 15, 2020
@jameshadfield
Copy link
Member

jameshadfield commented Mar 15, 2020

Hi @ncallaway -- thanks again for more great work here. This is really fantastic and allows us to credit all the contributions from translators. Breaking into functions is very wise. I've merged with one extra commit which addresses linting errors (c24aef6).

implement authors as arrays or strings, then share that code with the translators parsing.

This is great. I had separated this out into a separate issue (#934) which this PR will now close Whoops - #934 was slightly different, but this PR makes that issue such low priority that i'm closing it 🎉

The only documentation I found ...

Yeah - Unfortunately documentation is somewhat lacking for this (and not only this!). If you have ideas of how you think this should be done please let us know (via an issue).

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants