Page MenuHomePhabricator

Remove language sub-route from RPC doc permalinks
ClosedPublic

Authored by jasonbcox on Tue, Jul 28, 17:30.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC7778ab3bbd47: Remove language sub-route from RPC doc permalinks
Summary

While it would be nice to support translated documentation, this is unlikely to
happen in the near future. Cleaning up the /en from the permalinks makes the URLs shorter,
easier to remember, and more consistent with https://www.bitcoinabc.org/doc/dev/ which does
not have translation support (and is unlikely to ever have it, given the development overhead
of maintaining translated comments).

The /en was retained in the file paths since it does not negatively impact the build system
and it gives a hint for where translated RPC documentation could go in the future. When that
day comes, then we can consider how those translated docs fit into our routing scheme.

Test Plan

Build docs:

contrib/teamcity/build-configurations.py build-docs

Copy the docs over to the bitcoinabc.org repo:

mkdir -p ~/projects/bitcoin-abc-website/doc/
cp -r abc-ci-builds/build-docs/doc/rpc/en/0.21.13 ~/projects/bitcoin-abc-website/doc/

Build and deploy locally:

make container
make run

Navigate to http://127.0.0.1:8080/doc/0.21.13/ in your browser.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Tue, Jul 28, 17:30
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, Jul 28, 17:30
jasonbcox requested review of this revision.Tue, Jul 28, 17:30
jasonbcox retitled this revision from Remove langage sub-route from RPC doc permalinks to Remove language sub-route from RPC doc permalinks.Tue, Jul 28, 20:35
Fabien requested changes to this revision.Wed, Jul 29, 08:55
Fabien added a subscriber: Fabien.

I don't see the value of this diff. IMO a shorter URL (3 chars !) is not a strong case and this:

  • Makes the url inconsistent with the file system scheme,
  • Will make it more complicated to add translations, if someone is willing to do so.
This revision now requires changes to proceed.Wed, Jul 29, 08:55

I don't see the value of this diff. IMO a shorter URL (3 chars !) is not a strong case and this:

  • Makes the url inconsistent with the file system scheme,
  • Will make it more complicated to add translations, if someone is willing to do so.

Since we do not currently support a consistent routing scheme for translations, it's best to remove this from the permalinks until there is a concrete plan for translation support. Currently, these are the schemes smattered around the repo:

  • /<page> - default English page or post. A majority of pages on bitcoinabc.org are like this.
  • /<post-name>-<lang> - for non-English posts only
  • /en/<doc> - generate.go` is introducing a hard-coded third scheme for English that we do not currently support elsewhere.

Removing the /en prefix from permalinks today does push off the work for translations later, but adding it back should be trivial.
On the other hand, leaving /en in forces us to start making decisions about translations that we do not plan to support in the near-term. It's entirely possible that other schemes have better tradeoffs, so we should
focus our attention on new schemes later when we have time to dedicate to translations. For now, English-only without a dedicated URL scheme will work.

Fabien accepted this revision.Tue, Aug 4, 19:01

Talked offline, it is clear that setting the link is not the responsibility of this script and should be done on the website side.
Greening it for the sake of helping the tests in short term since it's not a regression per se, but the permalink should be removed from the generator at some point or at least rebuilt on the website side.

This revision is now accepted and ready to land.Tue, Aug 4, 19:01
This revision was automatically updated to reflect the committed changes.