replace `l10n.txt` include with link to `i18n.rst`

freedomofpress / securedrop-docs

Documentation for the SecureDrop project

https://docs.securedrop.org/

Other

22 stars 26 forks source link

replace `l10n.txt` include with link to `i18n.rst` #503

Closed cfm closed 11 months ago

cfm commented 11 months ago

Status

Ready for review

Description of Changes

Removes the l10n.txt include generated by securedrop's i18n_tool.py update-docs with a link to securedrop's i18n.rst, which is always authoritative for the set of languages we currently support and which GitHub will render automatically.

Towards, but can be reviewed and merged independently of, freedomofpress/securedrop#6917.

Testing

[ ] Visual review
[ ] b5f6073d31216096eef727770b40f40093501b2a is acceptable

Release

No special considerations.

Checklist (Optional)

[x] Doc linting (make docs-lint) passed locally
[x] Doc link linting (make docs-linkcheck) passed
[x] You have previewed (make docs) docs at http://localhost:8000

nathandyer commented 11 months ago

Code-wise, everything looks great here! That said, I'm honestly not sure how I feel about linking to a .json file in the documentation, rather than listing the supported languages directly. I just feel like if I were installing SecureDrop for the first time, and had to load a separate link and scan through a .json file to see the list of supported languages, I would be slightly put-off by it.

I personally would be in favor of enumerating the languages by hand, but I'm not dead-set on that, and would welcome discussion and differing opinions.

Testing

[x] Visual review
[ ] 1c82df5bdedcfedff3252c439d739eedd56a626c is acceptable

cfm commented 11 months ago

Thanks for letting me provoke your opinion, @nathandyer. :-) Would you be comfortable with either of the alternatives (2) or (3) in https://github.com/freedomofpress/securedrop/issues/6917#issuecomment-1689091116?

nathandyer commented 11 months ago

@cfm I think both (2) and (3) are good options, with a slight preference for (2). From my understanding, the result for the end-user would be the same (essentially) with either of those options, which is my main concern. That said, option (2) feels like a slightly cleaner implementation from a technical perspective.

cfm commented 11 months ago

Great. Tomorrow I'll tack that onto freedomofpress/securedrop#6954 and then update the link here.

cfm commented 11 months ago

This was a good call, @nathandyer. b5f6073d31216096eef727770b40f40093501b2a has the change, and I've left comments on the diff here to demonstrate GitHub's automatic rendering of the i18n.rst file.

cfm commented 11 months ago

b5f6073d31216096eef727770b40f40093501b2a works as expected after freedomofpress/securedrop#6954, so this is ready to go with your approval, @nathandyer.