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

Fix the language table in the docs #3493

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
@BrandonOCasey
Contributor

BrandonOCasey commented Aug 3, 2016

Description

Noticed that the language table on http://docs.videojs.com/docs/guides/languages.html is not correctly formatted. Looked at the markdown file for it and saw some malformed HTML. Fixed it

Requirements Checklist

  • Feature implemented / Bug fixed
  • Reviewed by Two Core Contributors
@mister-ben

This comment has been minimized.

Show comment
Hide comment
@mister-ben

mister-ben Aug 4, 2016

Contributor

Does this doc need to list language codes? Something like this from the explanation of Navigator.language on MDN might suffice:

A string representing the language version as defined in BCP 47. Examples of valid language codes include "en", "en-US", "fr", "es-ES", etc.

Contributor

mister-ben commented Aug 4, 2016

Does this doc need to list language codes? Something like this from the explanation of Navigator.language on MDN might suffice:

A string representing the language version as defined in BCP 47. Examples of valid language codes include "en", "en-US", "fr", "es-ES", etc.

@misteroneill

This comment has been minimized.

Show comment
Hide comment
@misteroneill

misteroneill Aug 4, 2016

Member

I'm all for leaving the table out - especially when there's a link directly to an always-updated list of files/languages. This seems like wasteful documentation.

Member

misteroneill commented Aug 4, 2016

I'm all for leaving the table out - especially when there's a link directly to an always-updated list of files/languages. This seems like wasteful documentation.

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Aug 4, 2016

Contributor

yeah that works too, I can just change it to a link

Contributor

BrandonOCasey commented Aug 4, 2016

yeah that works too, I can just change it to a link

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 4, 2016

Member

I think the original idea was to have the list of names for the json files in an easily accessible place.

Member

gkatsev commented Aug 4, 2016

I think the original idea was to have the list of names for the json files in an easily accessible place.

@misteroneill

This comment has been minimized.

Show comment
Hide comment
@misteroneill

misteroneill Aug 4, 2016

Member

The problem I see there is that you then have to remember to update this document anytime a new file is added/removed. Not that it's a common occurrence, but...

Member

misteroneill commented Aug 4, 2016

The problem I see there is that you then have to remember to update this document anytime a new file is added/removed. Not that it's a common occurrence, but...

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 4, 2016

Member

this list has all* the languages, not just the ones currently available in videojs.
Also, I wasn't necessarily saying we need to keep it, was just trying to give background on why it's there. If such a list is available in a more official place (specifically the two-letter language codes) we could just link it.

*not actually all, but a large selection of the most commonly used languages

Member

gkatsev commented Aug 4, 2016

this list has all* the languages, not just the ones currently available in videojs.
Also, I wasn't necessarily saying we need to keep it, was just trying to give background on why it's there. If such a list is available in a more official place (specifically the two-letter language codes) we could just link it.

*not actually all, but a large selection of the most commonly used languages

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
Contributor

BrandonOCasey commented Aug 4, 2016

@misteroneill

This comment has been minimized.

Show comment
Hide comment
@misteroneill

misteroneill Aug 4, 2016

Member

Oh, I see, then ignore me. :)

Member

misteroneill commented Aug 4, 2016

Oh, I see, then ignore me. :)

@BrandonOCasey

This comment has been minimized.

Show comment
Hide comment
@BrandonOCasey

BrandonOCasey Aug 4, 2016

Contributor

@gkatsev @mister-ben @misteroneill So the consensus here is to change the table into a link right? How about this?

Contributor

BrandonOCasey commented Aug 4, 2016

@gkatsev @mister-ben @misteroneill So the consensus here is to change the table into a link right? How about this?

removed the language table
link to an external website that contains all possible languages
@misteroneill

This comment has been minimized.

Show comment
Hide comment
Member

misteroneill commented Aug 5, 2016

LGTM

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Aug 8, 2016

Member

LGTM

Member

gkatsev commented Aug 8, 2016

LGTM

@gkatsev gkatsev added confirmed minor and removed needs: LGTM labels Aug 8, 2016

@gkatsev gkatsev modified the milestone: 3.12 build-improvements Aug 11, 2016

@gkatsev gkatsev closed this in 1bb40b8 Aug 11, 2016

@BrandonOCasey BrandonOCasey deleted the BrandonOCasey:fix/lang-table branch Aug 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment