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

Use Github Actions instead of Travis #121

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Use Github Actions instead of Travis #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6320f3
restore deleted info from github button
DO1JLR Feb 23, 2020
d72d4e0
Improve makefile
DO1JLR Feb 23, 2020
76283bd
Add github action to check the website
DO1JLR Feb 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/FUNDING.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# Information for Github sponsor button

github: [do1jlr]
Copy link
Contributor

Choose a reason for hiding this comment

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

We decided that it is probably the best solution to link to the contribution pages of both websites instead of private accounts to make things more transparent (see #119).

Can you remove your private account again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Wir" hatten das bei der einführung des spenden-buttons noch anders gehandhabt.

U.a. waren Teile des Vorstands damit einverstanden dies darin zu haben.

Copy link
Contributor

Choose a reason for hiding this comment

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

Grundsätzlich finde ich es besser, wenn die Spenden alle an eine zentrale Stelle gehen, das war schließlich der Grund warum wir die Sache mit dem Förderverein überhaupt angefangen haben, dass es nicht mehr so ein durcheinander gibt. Dachte eigentlich auch dass das deine Meinung ist nach der Nachricht neulich.

Nichtsdesdotrotz hat die Änderung hier auch nix mit der Anderen zu tun, muss also eh in einen anderen PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Du hast das auch alles in einem PR mit reingeschummelt btw

custom: ['https://ffbsee.net/helfen/', 'https://ffnbsee.org/verein/spenden/']
39 changes: 39 additions & 0 deletions .github/workflows/lektor-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: lektor check
on: [push, pull_request]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd probably only do this for pull-requests as branches could be quite "unready" and we already had some issues with running into quota limitations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Welche Issues hattest du denn?????

Copy link
Contributor

Choose a reason for hiding this comment

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

Die FFBsee orga hat nachwievor Probleme innerhalb des LFS Quota zu bleiben. Ich hab gesehen dass du versucht hast das dadurch zu lösen dass die Daten zu dem gitea bei see-base ausgelagert werden, aber das ist auch nur ein Workaround, da dann jeder einen Account bei see-base braucht und das Repo nun von zwei Servern abhängig ist.

Ich hab noch nicht nachgeschaut wie lange die Änderung mit LFS schon her ist, aber momentan ist das Quota noch stark überzogen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Da darfst du lange warten.

Das LFS Quota wird so schnell auch nicht wieder frei werden bis microsoft das umstellt!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dann schau nochmal nach und dann wirst du selber feststellen das diese aussage inhaltlich nicht ganz zutreffend ist!


jobs:
build:

runs-on: ubuntu-latest
strategy:
matrix:
python-version: [3.5, 3.6, 3.7, 3.8]
Copy link
Contributor

Choose a reason for hiding this comment

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

As the main purpose is whether the website still works, I don't think we need to test that many Python versions. The most recent stable is probably enough. Looking at my system this seems to be the 3.7 branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Und da wären wir schon bei Problemen.

Du magst altertümliche python versionen wie 3.7 verwenden. Ich verwende 3.8. Travis nimmt 3.6. Andere womöglich noch älter...

Lass uns doch python probleme erkennen in dem wir alle gängigen versionen testen!

Copy link
Contributor

Choose a reason for hiding this comment

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

Wir schreiben hier ja keine Bibliothek für Python, bei der wir ständig testen müssen ob die noch mit allen Versionen kompatibel ist. Prinzipiell würde es Sinn ergeben das mit der Version laufen zu lassen die von den Meisten die die Website bearbeiten verwendet wird, was momentan vermutlich die 3.7 ist, aber meinetwegen können wir auch schon die 3.8 nehmen, der Standard wird sich ja wahrscheinlich eh bald ändern, wenn der Support für 3.7 dieses Jahr abläuft.

Also sagen wir einfach schon 3.8.

Nachtrag: Ich sehe auch gerade das 3.8 noch garnicht auf der Lektor-Seite als "kompatibel getestet" steht. 🤷‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also eine offiziell nicht supportete pythonversion? Als single source of truth? sicher?


steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python -m pip install --upgrade pip
- name: Cache lfs
Copy link
Contributor

Choose a reason for hiding this comment

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

This step name seems to be a duplicate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ein update von pip tut nicht weh!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nicht die Zeile darüber, sondern diese. Der Name des Steps ist der Gleiche wie der darunter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dann markiere sie doch bitte korrekt.

Protipp: Es gehen auch multiline-markierungen!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ja gut, das sollte - wie man sieht - lektor heißen.

uses: actions/cache@v1
with:
path: ~/.cache/lektor
key: lektorcache
- name: Cache lfs
uses: actions/cache@v1
with:
path: .git
key: lfs
- name: install and pull lfs
run: |
sudo apt-get install git-lfs
git lfs pull
- name: install lektor
run: make install
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be sudo make install (also see comment on Makefile).

- name: build with lektor
run: |
make build
31 changes: 21 additions & 10 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,35 @@ all: build
.ONESHELL:
install:
if hash apt-get 2>/dev/null; then
sudo apt-get update -qq >/dev/null && sudo apt-get install -qq apt-utils imagemagick python3-pip python3-setuptools gcc
sudo apt-get update -qq >/dev/null && sudo apt-get install -qq apt-utils imagemagick python3-pip python3-setuptools gcc git-lfs
Copy link
Contributor

Choose a reason for hiding this comment

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

Makefiles which modify the system configuration usually do not have the sudo calls in them. Instead you are supposed to start make with sudo (or your system's equivalent) yourself to avoid surprising changes to user's systems. For make install this is usually the case. I'd prefer it if you could remove the sudo calls from the Makefile to avoid people accidentally modifying their system.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also: Couldn't we also install lektor from the package repositories instead of installing it globally with pip? I have the feeling that this might have unexpected behaviour for people actually using Python on their systems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Besser nicht.

Wenn wir die package repositories verwenden haben wir mal wieder potentiell komplett unterschiedliche lektor versionen oder im zweifelsfall einfach gar keine weil nicht im offiziellen repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makefiles which modify the system configuration usually do not have the sudo calls in them. Instead you are supposed to start make with sudo (or your system's equivalent) yourself to avoid surprising changes to user's systems. For make install this is usually the case. I'd prefer it if you could remove the sudo calls from the Makefile to avoid people accidentally modifying their system.

Das könnte man machen...
ABER die sudo calls sind schon vor diesem commit drinen und gehören doch eher in einen neuen Pull-Request! Daher nein.

Copy link
Contributor

Choose a reason for hiding this comment

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

Naja, du änderst ja sowieso schon einige Sachen hier in der Datei, damit das mit der neuen CI gut funktioniert, da kann man diese Änderung auch gleich mitnehmen, das ist denke ich themenverwandt.

elif hash pacman 2>/dev/null; then
sudo pacman -Syu imagemagick python-pip glibc lib32-glibc gcc --noconfirm
sudo pacman -Syu imagemagick python-pip glibc lib32-glibc gcc git-lfs --noconfirm
elif hash dnf 2>/dev/null; then
sudo dnf install -y ImageMagick python3-pip gcc
sudo dnf install -y ImageMagick python3-pip gcc git-lfs
else
echo -e "Please install Imagemagick, python3-pip and gcc"
echo -e "Please install Imagemagick, python3-pip git-lfs and gcc"
fi
pip3 install --user --upgrade wheel
pip3 install --user --upgrade lektor
sudo pip3 install wheel --upgrade
sudo pip3 install lektor --upgrade

build:
lektor clean --yes
lektor plugin flush-cache
lektor build $(LEKTOR_PLUGIN_FLAGS)
if python3 -m lektor --version 2>/dev/null; then
xperimental marked this conversation as resolved.
Show resolved Hide resolved
python3 -m lektor build $(LEKTOR_PLUGIN_FLAGS)
else
lektor build $(LEKTOR_PLUGIN_FLAGS)
fi

server:
lektor server $(LEKTOR_SERVER_FLAGS)
if python3 -m lektor --version 2>/dev/null; then
python3 -m lektor server $(LEKTOR_SERVER_FLAGS) $(LEKTOR_PLUGIN_FLAGS)
else
lektor server $(LEKTOR_SERVER_FLAGS) $(LEKTOR_PLUGIN_FLAGS)
fi

deploy:
lektor clean --yes
lektor plugin flush-cache
lektor build $(LEKTOR_PLUGIN_FLAGS) $(LEKTOR_DEPLOY_FLAGS)

server-all:
lektor clean --yes
Expand Down