Skip to content

Conversation

acuteenvy
Copy link
Member

This PR does not break anything; the old method of distributing assets is still there.

My proposal for fixing this issue is: we merge this PR, then announce that we are moving the assets from https://raw.githubusercontent.com/tldr-pages/tldr-pages.github.io/main/assets (https://tldr.sh/assets) to https://github.com/tldr-pages/tldr/releases/latest/download (and I think we should not make redirects so that we only have one URL). After some time, we swap the website repositories as described in #12048 (comment) and remove the old distribution method from deploy.sh.

If someone comes up with a better solution, I'll just close this PR.

Discussed in #12048.

@acuteenvy acuteenvy added the decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. label Jan 11, 2024
@github-actions github-actions bot added the tooling Helper tools, scripts and automated processes. label Jan 11, 2024
Copy link
Member

@sebastiaanspeck sebastiaanspeck left a comment

Choose a reason for hiding this comment

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

LGTM, maybe store "$SITE_HOME/assets" as well in a variable? If we change the folder in the future, it is only one place to edit.

Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@acuteenvy
Copy link
Member Author

I found an issue with this - when we create a new release (after a client spec update), it has no assets at first. That would cause clients to return 404 errors until the assets are deployed, and we only deploy assets for languages that were changed in the commit. To fix this, I've added a workflow that copies the assets from the previous release. I'll upload the first batch of assets manually after this PR gets merged.

Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

Adding to the changes, instead of publishing assets to regular releases every time (how about setting up a common continuous build release? (We use a similar system for development builds in another project I am involved in; although with a separate action [gh release would work similarly too], we can mark it as pre-release so that it doesn't show up over the client specification ones).

This would act as a common place to fetch assets for all clients, the underlying tag being outdated isn't an issue as the assets can be updated anytime independently. This would reduce the burden for most clients (as they can implement client specification changes independently without worrying about showing outdated pages or missing pages until they update to the new release).


Using the request changes option to prevent accidental merge of this PR, before discussing this.

@acuteenvy
Copy link
Member Author

This would reduce the burden for most clients (as they can implement client specification changes independently without worrying about showing outdated pages or missing pages until they update to the new release).

This is not a problem. https://github.com/tldr-pages/tldr/releases/latest/download/tldr-pages.en.zip redirects to tldr-pages.en.zip from the latest release. Clients will never need to update the URL.

@kbdharun
Copy link
Member

This is not a problem. https://github.com/tldr-pages/tldr/releases/latest/download/tldr-pages.en.zip redirects to tldr-pages.en.zip from the latest release. Clients will never need to update the URL.

Agreed, that works too. Will dismiss my review. IG the PR is GTG.

Copy link
Member

@sbrl sbrl left a comment

Choose a reason for hiding this comment

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

Am I right in thinking that this pushes to the same (pre)release each time? This would be preferable to:

  1. Avoid lots of releases drowning out the client spec updates
  2. Avoid lots of disk space being used and GitHub getting mad at us

Otherwise, looks good to me! I think we can merge this and give it a try~~~ :D

@kbdharun
Copy link
Member

kbdharun commented Jan 27, 2024

Am I right in thinking that this pushes to the same (pre)release each time? This would be preferable to:

  1. Avoid lots of releases drowning out the client spec updates
  2. Avoid lots of disk space being used and GitHub getting mad at us

Otherwise, looks good to me! I think we can merge this and give it a try~~~ :D

It doesn't push to a pre-release, but rather the current latest release. Since releases aren't Git based there aren't any soft bandwidth limits for updating but files do have a size limit of 2 GB (Which I am sure we won't hit in a very long time 😁).

Checkout https://docs.github.com/en/repositories/releasing-projects-on-github/about-releases.

@kbdharun kbdharun merged commit 4da50c1 into tldr-pages:main Jan 28, 2024
@kbdharun
Copy link
Member

I found an issue with this - when we create a new release (after a client spec update), it has no assets at first. That would cause clients to return 404 errors until the assets are deployed, and we only deploy assets for languages that were changed in the commit. To fix this, I've added a workflow that copies the assets from the previous release. I'll upload the first batch of assets manually after this PR gets merged.

Done, merged. Let's see how it goes.

@acuteenvy
Copy link
Member Author

I've uploaded the rest of the assets. Everything should work fine now.

@sbrl
Copy link
Member

sbrl commented Feb 6, 2024

@acuteenvy, @kbdharun: Looks like it worked, but I don't see a .zip for English:

image

@kbdharun
Copy link
Member

kbdharun commented Feb 7, 2024

Looks like it worked, but I don't see a .zip for English

It is indeed present @sbrl (at the last 😅); GitHub appends recently updated assets from the last and it can be seen by expanding the list (since it isn't alphabetically sorted IG you missed it).

image

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

Labels

decision A (possibly breaking) decision regarding tldr-pages content, structure, infrastructure, etc. tooling Helper tools, scripts and automated processes.

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants