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.

Sign up for GitHub

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

Jump to bottom

GitHub actions & Buildzeit-Optimierungen #161

Closed
wants to merge 6 commits into from
Closed

GitHub actions & Buildzeit-Optimierungen #161

wants to merge 6 commits into from

Conversation

ingsie
Copy link

@ingsie ingsie commented Aug 13, 2022

Optimierung des Buildzeit durch Wechsel von rasterize.js auf rsvg-convert:

  • Reduktion der Zeit zur Erzeugung der PNG auf meinen Testsystemen um 90%, von ca. 30min auf ca. 3 min auf dem Schwächeren.
  • Entfernung von optipng, da die PNG-Dateien aus rsvg-convert nur unwesentlich größer als die mit optipng optimierten sind.

Github Actions:

  • Automatischer Build und Erstellung eines Entwicklungsreleases nach Push oder Pull request in den master
  • Automatischer Build und Erstellung eines Releases beim Push eines Tags, dessen Name mit 'v' beginnt. Die auf das "v" folgende Versionsnummer muss eine gültige semantische Versionsnummer sein (vgl. https://semver.org/lang/de/)

Anpassung von symbols/Fernmeldewesen/Bedingung_DMO.j2 zur Beseitigung von Darstellungsfehlern in den resultierenden PNG.

kmpoppe
kmpoppe reviewed Aug 14, 2022
View reviewed changes
Copy link

@kmpoppe kmpoppe left a comment

Choose a reason for hiding this comment

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

Kommentare zu zwei Files, ansonsten sieht das für mich auf den ersten und zweiten Blick sehr gut aus!

symbols/Fernmeldewesen/Bedingung_DMO.j2
@@ -1,5 +1,5 @@
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" height="256">
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="256" height="256" viewbox="0 0 256 256">
Copy link
Author

Choose a reason for hiding this comment

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

Die selbe Anpassung ist schon in #150 enthalten, ggf. hier reverten um merge conflict vorzubeugen?

Ich vermute du meinst #152. Wenn #152 vor diesem PR integriert wird, wäre ein Revert unproblematisch, andernfalls wäre nach dem Merge dieses PR taktische Zeichen "Bedingung_DMO" nicht als brauchbare PNG-Version verfügbar.

Copy link

Choose a reason for hiding this comment

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

Wohl wahr. Resolved.

ingsie
ingsie commented Aug 14, 2022
View reviewed changes
symbols/Fernmeldewesen/Bedingung_DMO.j2
@@ -1,5 +1,5 @@
<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" height="256">
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" width="256" height="256" viewbox="0 0 256 256">
Copy link
Author

Choose a reason for hiding this comment

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

Die selbe Anpassung ist schon in #150 enthalten, ggf. hier reverten um merge conflict vorzubeugen?

Ich vermute du meinst #152. Wenn #152 vor diesem PR integriert wird, wäre ein Revert unproblematisch, andernfalls wäre nach dem Merge dieses PR taktische Zeichen "Bedingung_DMO" nicht als brauchbare PNG-Version verfügbar.

@ingsie @kmpoppe
Update README.md
b6728de 
Slight rewording for better readability. Removed PhantomJS Ubuntu reference as it was removed from the necessary programs.

Co-authored-by: Kai Michael Poppe <[email protected]>
@jonas-koeritz
Copy link
Owner

GitHub Actions ist inzwischen implementiert.

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

@kmpoppe kmpoppe kmpoppe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ingsie @jonas-koeritz @kmpoppe