-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Improve CI process for releases #892
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[why] Fonttools, FreeType, and HarfBuzz are not used in the CI job. Propably a leftover from thomething done in the past? [how] Because I can not find out the reason WHY they are there the lines are kept and just disabled. This leaves a nice 'stub' for git blame in the file. Signed-off-by: Fini Jastrow <[email protected]>
[why] The self-written upload-archives script is some issues, for example it does not replace preexisting release files with new ones when a release is re-triggered. The error messages are rudimentary at best. [how] Use https://github.com/softprops/action-gh-release instead. Signed-off-by: Fini Jastrow <[email protected]>
[why] This can not work. Issue 1: It is unspecified in which order the font matrix jobs run, so the version number changes somewhen. The changed (version-bumped) files are never committed and written back, so this would have to be done manually anyhow. The version-bump script itself has issues because the regexes for the change are a bit too loose and other version like strings are changes also (like the Script Version). Issue 2: The script changed versions that should not be changed like the script version in the scripts (rather than the NF release version). In bin/scripts/generate-glyph-info-from-set.py pumping has been forgotten/omitted completely. [how] In the version-bump script: * Use more modern regex * Instead of copying the code use a loop Create a commit with the bumped version information in all scripts. This can only be done with the final job, because we have a problem to checkout the modified version with actions/checkout. We need to bump the version on every patch job, because we need the correct information already in the script when we patch. [note] This does not prevent to have multiple commits attempts that change to the same version. But if the change is empty, no commit will be added and the step is silently ignored. Signed-off-by: Fini Jastrow <[email protected]>
[why] We show only the Release version of the patcher script. Often it is unclear which particular script people are using. [how] Also show the script version. This is a bit frickle, as it needs developers to change the number manually (as with all other scripts in the project that have a script version), but so be it. This could be replaced by the 'current' git hash. But that has the drawback that one needs to search for the commit to see how old the script is. Signed-off-by: Fini Jastrow <[email protected]>
[why] We need the release tag version in all jobs. [how] Instead of determining it in every job again and again (with the same code) we set an output in the first job and reuse it everywhere. Signed-off-by: Fini Jastrow <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
[why] We check the whole repo out, just to set up the font matrix. All data is thrown away afterwards. That takes needless time. [how] Just check out the two files we really need for the setup (the script and the database). Signed-off-by: Fini Jastrow <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
[why] When people check the repo out at a release tag they expect to get all the files that are IN the release. But we add the patched fonts and the version-bumped scripts only afterwards. [how] Just overwrite (shift) the tag after everything is finished. Signed-off-by: Fini Jastrow <[email protected]>
[why] The release CI is run every time something is pushed to master. This is useful if we are on a release candidate. The release will be updated. But if we do a normal release and afterwards push some new commit to master - before we update the version in the package.json to a RC - the actual (fixed) release will also be updated. [how] Determine if we have a * new normal release * new prerelease * updated prerelease and run the release process itself only in that cases. [note] The release is checked for via API instead of an action, because the typical action needs a complete checkout (works on the local git repo). Signed-off-by: Fini Jastrow <[email protected]>
[why] When run on more than one font the mini-readme is only added to the first created archive. [how] Remove the readme file only after all archives have been generated. [note] Also rewrite `find` command, to actually do some work. The formulation was rather odd, using a shell glob to find the directories and `find` to select one. Signed-off-by: Fini Jastrow <[email protected]>
[why] The CI uses the script with the font-matrix name, to regenerate the cask of just one font. But the script is ignoring the parmeter and generates all casks. [how] Allow to specify a regex pattern as first parameter and limit the processing to that font(s), as done in archive-fonts.sh Signed-off-by: Fini Jastrow <[email protected]>
[why] The script shows # [Nerd Fonts] Generating cask for: . for any font it processes. [how] Well, the variable naming is a bit strange, maybe that is the reason for the bug? We already have the font name in another variable, use that. Signed-off-by: Fini Jastrow <[email protected]>
[why] The filter expects 'Mono' to be the last part of the font filename. With font-patcher's --makegroups that is not the case anymore, because in fact 'Mono' is part of the font name and the font filename ends in the style (i.e. 'Italic'). [how] Because we do only create 'Complete' fonts, and 'Mono' is always after 'Complete' (i.e. 'Complete Mono') for both veriants created by the font-patcher, we can use that to select mono fonts. [note] Also bump script version after several changes. Signed-off-by: Fini Jastrow <[email protected]>
[why] Even when the logical content does not change we will get a new commit on recreation because the order can/will be different. [how] Put the fonts in alphabetical order in the config file. Signed-off-by: Fini Jastrow <[email protected]>
[why] The fontconfig and cask generation seems unfinished or broken. [how] Fix the point in time when the fontconfig is generated and commit any changes back to the repo. Generate the casks and store them as CI artifacts at least. Probably they should be uploaded to the cask repo (on 'real' releases)? Signed-off-by: Fini Jastrow <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
[why] The release workflow is triggered on a lot occasions, but not on changes of the package.json file. But that file contains our version number and when we change it we should create a new release. At least is that how I would imagine it working. Signed-off-by: Fini Jastrow <[email protected]>
[why] Usually release tags and versions start with a "v", and we also had that up to 2.1.0: $ git tag -l 2.2.0-RC FontPatcher v0.1.0 v0.1.1 v0.1.2 v0.2.0 v0.2.1 v0.3.0 v0.3.1 v0.4.0 v0.4.1 v0.5.0 v0.5.1 v0.6.0 v0.6.1 v0.7.0 v0.8.0 v1.0.0 v1.1.0 v1.2.0 v2.0.0 v2.1.0 [how] In the files we keep the non-"v" version numbers, but the tags on github shall be prepended with a "v" like "v2.2.0-RC". The variable RELEASE_TAG_VERSION is renamed to RELEASE_VERSION to make clear that this is not the name of the tag. Where we reference a tag, "v$RELEASE_VERSION" is used. [note] One of the environment variable is not needed, we can use the output directly. Signed-off-by: Fini Jastrow <[email protected]>
[why] `grep` and `awk` do not know json. This might break if some format changes or whatever. [note] The font matrix setup is also with `jq`. Signed-off-by: Fini Jastrow <[email protected]>
[why] The docker image should be rebuilt whenever a file that will end up in the container has been changed. So this needs to be in line with the .dockerignore file. [how] Add missing (new) `font-patcher` sub-scripts. Signed-off-by: Fini Jastrow <[email protected]>
[why] On forks for example they might have a docker repo associated or not. The release process is run in any case and will fail in that cases. [how] Check that at least the secret is known. Signed-off-by: Fini Jastrow <[email protected]>
LNKLEO
pushed a commit
to LNKLEO/Nerd
that referenced
this pull request
Nov 24, 2023
Improve CI process for releases With merge request to have the complete overhaul in a closed context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While preparing the
2.2.0
release I found several shortcomings in the current process.This is a loose collection of all the changes that resulted in the process.
There are a lot changes, but I tried to break them down into digestible chunks / commits.
Requirements / Checklist
What does this Pull Request (PR) do?
master
package.json
and pushing it tomaster
v
(e.g.v2.3.0-RC
orv3.0.0
) (but not the number inpackage.json
)The release process will:
bin/scripts/lib/fonts.json
)Agave.zip
) [*]cask
s for the archives (as artifact, not further processed)font-patcher.zip
archive on the release page [*]10-nerd-font-symbols.conf
patched-fonts/
[*] Note that steps 3, 5, and 10 are ommitted when there is no release candidate (no running release) and we already have a (immutable) release. Then just the files in the repo are updated, but nothing can be seen on the release page.
How should this be manually tested?
Apart from the checks that the release files contain useful data the logic behind the releases has to be tested:
Note that the tests need to be done on the
master
branch to check the automatic workflow triggers.Probably a fork is a good solution.
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)
Test as described above (with a small font subset).
Step 1
Create a new release by:
https://github.com/Finii/nerd-fonts/actions/runs/2932178246
Note the outputs that show exactly what kind of release has been detected and if it is immutable or not:
Step 2
Step 3
https://github.com/Finii/nerd-fonts/actions/runs/2932292079
Some steps are disabled now, for example:
Checking the release page, it is unchanged. Well the files are not updated. But the repo is:
Step 4
https://github.com/Finii/nerd-fonts/actions/runs/2932455976
Step 5
Note: This is not the correct image, it is in fact taken after Step 6
Step 6
https://github.com/Finii/nerd-fonts/actions/runs/2932617566
The rolling release / release candidate is shifted,. The circled items will show different values than before:
Example git log (not complete)
![image](https://user-images.githubusercontent.com/16012374/186885742-e749efd1-6597-4e29-8549-9eae8a22be43.png)