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

No bleeding edge FontPatcher.zip #1040

Closed
Finii opened this issue Jan 9, 2023 · 6 comments · Fixed by #1044
Closed

No bleeding edge FontPatcher.zip #1040

Finii opened this issue Jan 9, 2023 · 6 comments · Fixed by #1044

Comments

@Finii
Copy link
Collaborator

Finii commented Jan 9, 2023

in order to avoid the 30 GB of monorepo history and whatnot from here

Well, in the meantime we have a single zip file that holds the last release script with all additional files needed for patching:
https://github.com/ryanoasis/nerd-fonts/releases/download/v2.3.0-RC/FontPatcher.zip

But well, that is not master. And it is very badly documented.
We need to

  • promote that more (who wants to write docs?)
  • install some CI that keeps one version of that zip as 'rolling release' on master

Originally posted by @Finii in #595 (comment)

@b- b- mentioned this issue Jan 9, 2023
2 tasks
@b-
Copy link
Contributor

b- commented Jan 9, 2023

I mentioned this in the PR, but it's marked as a draft because the step performing the actual release is most certainly not correct for this repository. I'd like to ask for help with that part, but I'm pretty sure the zip file this spits out is correct.

It's a little bit rough in that I'm checking out the unpatched fonts and then deleting them, rather than being more specific about what to checkout, but I don't know if this is such a big deal. At the very least I'm not checking out the whole huge repo and its eternal(ly large) history

@b-
Copy link
Contributor

b- commented Jan 9, 2023

Now that I think about it, it probably would make sense to have the action simply add the zip to the repo and push that. I think that's what you're suggesting, @Finii ?

I'm not certain how to add to such a bare repo, though. Maybe we can just cp fontpatcher.zip nerd-fonts/fontpatcher.zip && git add fontpatcher.zip && git commit -m"Update fontpatcher.zip" && git push?

Edit: (not with the removed .git directory, but let me try this)

@Finii
Copy link
Collaborator Author

Finii commented Jan 9, 2023

Yes.

Thanks for the RP.
I'm almost in bed already, just some fast rough notes...

You should use actions instead of writing your own git commands. Like https://github.com/actions/checkout
See how the already existing workflows do it.

There is already a script that creates the release zip file, why not use that for inter-release zips?
https://github.com/ryanoasis/nerd-fonts/blob/master/bin/scripts/archive-font-patcher.sh

Of course there are also actions to commit a new file to the repo. Like, see
https://github.com/ryanoasis/nerd-fonts/blob/master/.github/workflows/packsvgs.yml

One would need to think about the trigger more closely.

If you want to test the workflow I would recommend forking this repo and doing the experiments in your fork.

@b-
Copy link
Contributor

b- commented Jan 9, 2023

Thanks for the notes! I'll work on them.

You should use actions instead of writing your own git commands. Like https://github.com/actions/checkout See how the already existing workflows do it.

I was annoyed by how slow actions/checkout is, but indeed I see in the existing workflows the following:

# Faster version instead of - uses: actions/checkout@v3
      - uses: Bhacaz/checkout-files@v2
        with:
          files: package.json bin/scripts/get-font-names-from-json.sh bin/scripts/lib/fonts.json
          branch: ${{ github.head_ref || github.ref_name }}

So thanks for pointing that out, this does feel a lot cleaner than my own git commands (though I do prefer understanding under the git hood more than the magic of asking someone else for help...). Of course I can adapt files: to files: font-patcher src/glyphs src/svgs src/archive-font-patcher-readme.md <...>

There is already a script that creates the release zip file, why not use that for inter-release zips? https://github.com/ryanoasis/nerd-fonts/blob/master/bin/scripts/archive-font-patcher.sh

Checking that out now!

Of course there are also actions to commit a new file to the repo. Like, see https://github.com/ryanoasis/nerd-fonts/blob/master/.github/workflows/packsvgs.yml

This also looks like it'll help, thanks.

One would need to think about the trigger more closely.

May I ask you to elaborate, please? The token automatically used by GitHub Actions will not automatically trigger other actions (by design, so that a naively written action won't spawn a loop). Is it bad to take advantage of this? Were there other issues you had in mind about? I guess there's probably a way to filter the on.push trigger to only run when either /font-patcher or /src/glyphs are updated, looking at archive-font-patcher.sh

If you want to test the workflow I would recommend forking this repo and doing the experiments in your fork.

Way ahead of you -- I've got actions enabled in my own fork, so all of my commits automatically test this PR.

@Finii
Copy link
Collaborator Author

Finii commented Jan 9, 2023

May I ask you to elaborate, please? The token automatically used by GitHub Actions will not automatically trigger other actions (by design, so that a naively written action won't spawn a loop). Is it bad to take advantage of this?

Well, I would think the zip should automagically be created when something is pushed to master, like the docker release is also following HEAD rather then the releases.

Iirc you have a push trigger, but that will be called on every push, also push into branches.

Alternatively, if we have a release candidate, we could update the zip file of that RC, and we do not need to mess around with the actual repo (which is kind of ... smelly in this case?) Maybe that is even better. Check if the last release is a RC, update the FontPatcher.zip if it is. Do nothing if we do not have a RC? Hmm.

Way ahead of you -- I've got actions enabled in my own fork, so all of my commits automatically test this PR.

🤔 Ah, I see now... Usually I PR only after I'm finished and all works, to not 'spam' the PR with all my tests.

@b- b- mentioned this issue Jan 10, 2023
2 tasks
@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants