-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Release master fontpatcher #1044
Conversation
Round 2, with less spam |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very good!
I have some questions and suggestions, see below.
And then, I would not put FontPatcher.zip
into the PR, because it will be outdated anyhow when the PR is pulled. It can be added after pulling by triggering the workflow.
Another thing, maybe we should do a zipcmp
after creating the zip file to see if the contents of the zip really changed, or just the timestamps differ. At the moment someone could push 'run this workflow' multiple times and we get a new commit always.
name: Release font patcher from master | ||
|
||
on: | ||
create: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why we want to trigger on create
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was for my own testing, forgive me.
steps: | ||
|
||
- name: Grab the script and its dependencies from the repo | ||
uses: snow-actions/sparse-checkout@v1.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other workflows with equivalent step we use Bhacaz/checkout-files@v2
.
Maybe use that here also or also switch the other ones?
Can you give details? The less actions we use the easier are updates ;)
I guess this one uses patterns while the other one needs concrete files (but I did not check).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my below conversation: the action you use to checkout in the other actions, at least I think that’s the checkout action I am thinking of, messed with/reset permissions by grabbing files using GitHub’s HTTPS api. This one uses good ol’ git under the hood.
I think it would be less “dirty” to use a git
(the CLI)-based action like this whenever possible (ie., maybe indeed switch other workflows to it?), and you can see in my action runs it still is pretty fast for this big clunker of a repo
That all was the reason I picked another action, anyway — you (understandably) didn’t like the manual git command twiddling I’d started with, but I really didn’t want to clone even the entire worktree when we are just using a small section of it.
- run: chmod +x | ||
font-patcher | ||
bin/scripts/archive-font-patcher.sh | ||
bin/scripts/name_parser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the action does not restore the access rights?
If you want to +x
the scripts in name_parse
is not some -R
option or a glob (*
) needed for chmod
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this was a mistake on my part for something else — I have to double check, but I think a different action I used previously to clone the repo used tarballs under the hood and didn’t set permissions properly.
We should be able to just remove the line and not have a problem:
This checkout is just a real sparse checkout (it’s great, I love sparse checkout, it’s the only way to wrangle big repos like https://GitHub.com/QMK/qmk) so it shouldn’t mess with the permissions.
with: | ||
fetch: false | ||
add: "FontPatcher.zip" | ||
message: "[ci] update FontPatcher.zip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always start capitalized (i.e. s/update/Update/
), but I can also change that.
@@ -0,0 +1,45 @@ | |||
name: Release font patcher from master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can find a better name here (also for the yaml file).
Esp this is not a 'release' but some snapshot.
Maybe Create font-patcher snapshot
and font-patcher-snap.yml
? Hmm 🤔
Maybe Create font-patcher archive
and font-patcher-zip.yml
?
All other workflows have the .yml
suffix, I think we should stick with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of the two you suggested, I like
Maybe Create font-patcher archive and font-patcher-zip.yml?
more. “Snap” gives me these uncomfortably weird Ubuntu tinglies
branches: master | ||
paths: | ||
- font-patcher | ||
- src/glyphs/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone pushes a fixed svg
to src/svgs
the packsvgs.yml
will run and create a new original-source.ttf
and push it. But I guess that will NOT trigger this workflow unless one adds something somewhere. I have no clue what and where. Just that there are some ... hurdles if workflow commits shall trigger workflows 🤔
It is ok how it is now, but maybe you know the answer :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also after a release.yml run, I guess the zip inside the repo will not be updated although the version in the script has been changed (by the release workflow) 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone pushes a fixed
svg
tosrc/svgs
thepacksvgs.yml
will run and create a neworiginal-source.ttf
and push it. But I guess that will NOT trigger this workflow unless one adds something somewhere. I have no clue what and where. Just that there are some ... hurdles if workflow commits shall trigger workflows 🤔It is ok how it is now, but maybe you know the answer :-)
I don’t remember if this step actually functioned or not — I might have been missing something when I set it up. But the idea here is to set an API PAT (Personal Auth. Token) for the repository you want to trigger workflows on, add it as a secret to the repo you want to trigger workflows from, and then just call the second workflow from the first.
I’ve done some research, and I don’t think it’s possible to trigger one workflow from another without a custom PAT. Actions Workflows get a token by default anyway, but it specifically has the ability to trigger other workflows disabled — that’s the issue there
@b- |
Oh jeez, I wanted to run it again because I wanted to patch a font, and I honestly forgot why it wasn't merged. Let me remove the zip right now! |
I agree a zipcmp would be a good idea. I just removed the FontPatcher.zip from the repo, but I want to use a separate branch to figure out the zipcmp part. |
Okay, we got a zipcmp in there now! I put the conditional on the step level, but I'm not sure if it would be preferable to have a separate job with a step to do the release. Gut feeling suggests to me that it would be, especially what with the pretty pipeline graphics that you see when you have multiple jobs in an actions pipeline. But I had some trouble with the outputs/conditional on the job level, and this definitely works just fine as is. I also am a little on the fence because using separate jobs means that the runner has a clean slate on the subsequent job, which means duplicating the checkout step and adding artifact upload and download steps. |
e7a73ad
to
f42e35f
Compare
|
8c0ddb0
to
02f807e
Compare
I changed the workflow a bit, hope you do not mind. |
[why] It is not really a release, it is just a convenience for people that want to download the current master patcher with all accessories. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] Some changes to bring it in line with the other workflows. [how] Sorted from short to long: * Name all steps * Correct trigger * Remove unused output * Simplify zipcmp decision * Use apt instead of apt-get * Update upload-artifact action * Remove some trailing whitespace * Remove name from artifact to prevent strange double zip name * Use ordinary checkout instead of sparse - we need it anyhow to commit [note] Ok, 'simplify zipcmd decision' that might be matter of taste. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
[why] If we create the zip file not only on releases we need some better version information inside. We can not use the same approach with real releases, because we add the tag at a later stage in the workflow. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
02f807e
to
31cae9d
Compare
Hmm, somehow the rebase on master was lost. |
@all-contributors add @b- for infrastructure |
I've put up a pull request to add @b-! 🎉 |
Fixed via afa6abe |
That made my night to read, thank you! |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ryanoasis/nerd-fonts](https://togithub.com/ryanoasis/nerd-fonts) | major | `v2.3.3` -> `v3.0.0` | --- ### Release Notes <details> <summary>ryanoasis/nerd-fonts</summary> ### [`v3.0.0`](https://togithub.com/ryanoasis/nerd-fonts/releases/tag/v3.0.0) [Compare Source](https://togithub.com/ryanoasis/nerd-fonts/compare/v2.3.3...v3.0.0) ***Update: Preparation already for a bugfix release, see known bugs [here](https://togithub.com/ryanoasis/nerd-fonts/milestone/26)*** This major release introduces some breaking changes: ##### Breaking 1: Naming This release fixes some long standing issues that are due to the naming of the fonts: There is a completely new naming scheme. This might be inconvientient for existing setups, sorry. - Some fonts will have `Nerd Font` in the name while other have it abbreviated as `NF`. This is needed because some names just were too long. (Same for `Nerd Font Mono` and `NFM`.) - There are no `Windows Compatible` fonts anymore. All fonts work on all platforms. - The `Complete` from the font names (and the repo directories) has been dropped (Complete is the new normal). - The name parts will be ordered as expected with style and weight last (`Somefont Bold Nerd Font` -> `Somefont Nerd Font Bold`). - The filename will have no blanks anymore. ##### Breaking 2: Material Design Icons Codepoints The old Material Design Icon codepoints are finally dropped. Due to an historic mistace we placed them in between some asiatic glyphs, breaking that script. Since v2.3.0 the (updated and expanded) Material Design Icons have new codepoints in the 5 digit region. - Dropped codepoints `F500`... and class names `nf-mdi-*` - New codepoints `F0001`... and class names `nf-md-*` (already since `v2.3.0`) - The whole discussions are here: [https://github.com/ryanoasis/nerd-fonts/issues/365](https://togithub.com/ryanoasis/nerd-fonts/issues/365) - A translation table is available here: [https://github.com/ryanoasis/nerd-fonts/issues/1059#issuecomment-1404891287](https://togithub.com/ryanoasis/nerd-fonts/issues/1059#issuecomment-1404891287) - There are tools out there that probably can update your configuration. - Relevant thread: [#​1190](https://togithub.com/ryanoasis/nerd-fonts/issues/1190) Otherwise this is a 'Update release', so now the good stuff: ##### Updates - Update `Agave` to v37 - Update `Arimo` to 1.33 - Update `DaddyTimeMono` to 1.2.3 - Update `Fira Mono` to 3.206 - Update `Go Mono` to 2.010 - Update `Hermit` to 2.0 - Update `IBM Plex` to 2.3 - Update `Iosevka` to 22.1.0 - Update `Literation` to 2.1.5 - Update `Lilex` to 2.000 - Update `mononoki` to 1.6 - Update `MPlus` to ... current - Update `Overpass` to 3.0.5 - Update `Roboto Mono` to 3.0 - Update `Source Code Pro` to 2.038 - Update `Terminus` to 4.49.2 - Update `Victor Mono` to 1.5.4 - Update the Octicons set to 18.3.0 ##### Features - New font `ComicShannsMono` - New variant in release `Nerd Font Propo` for GUI usecases - Patch in heavy angle brackets - Patch in boxdrawing glyphs (if the font has no complete set) - Repair Panose info if source font has broken data - Reform PowerlineExtra sizing - Autocreate a `FontPatcher.zip` from `HEAD` - Create a json database with css names - Disentangle `Iosevka` into two packets (one for `Iosevka Term`) - Add option to manipulate `xAvgCharWidth` (needed rarely by self-patchers) - Add option to allow italic-less fonts with oblique - Add `--debug` and `--dry` to `font-patcher` - Add logging into file to `font-patcher` - Add `NERDFONTS` environment variable to transport options through `gotta-patch-em` ##### Improvements - Fix `Caskaydia Code` height different to `Cascadia Code` (hinting problem) (font is now `ttf` instead of `otf`!) ##### Fixes - Fix baseline to basline distance (line gap) for some fonts - Fix weather icons cloud scaling - Fix UniqueID of the fonts - Fix `Bitstream Vera` name: Is now `Bitstrom Wera` due to licensing issue - No fix, but: Drop support for Python 2 #### New Contributors - [@​Goooler](https://togithub.com/Goooler) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1079](https://togithub.com/ryanoasis/nerd-fonts/pull/1079) - [@​MicaelJarniac](https://togithub.com/MicaelJarniac) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1100](https://togithub.com/ryanoasis/nerd-fonts/pull/1100) - [@​teatimeguest](https://togithub.com/teatimeguest) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1119](https://togithub.com/ryanoasis/nerd-fonts/pull/1119) - [@​b-](https://togithub.com/b-) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1044](https://togithub.com/ryanoasis/nerd-fonts/pull/1044) - [@​Weltolk](https://togithub.com/Weltolk) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1163](https://togithub.com/ryanoasis/nerd-fonts/pull/1163) - [@​sullrich84](https://togithub.com/sullrich84) made their first contribution in [https://github.com/ryanoasis/nerd-fonts/pull/1166](https://togithub.com/ryanoasis/nerd-fonts/pull/1166) **Full Changelog**: ryanoasis/nerd-fonts@v2.3.3...v3.0.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/scottames/dots). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS43MS4wIiwidXBkYXRlZEluVmVyIjoiMzUuNzEuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Create FontPatcher.zip in repo up-to-date with master
Description
Fixes #1040
@all-contributors please add @b- for infra
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
#1041
What are the relevant tickets (if any)?
#1040
Screenshots (if appropriate or helpful)