stefanhaustein / TerminalImageViewer

Small C++ program to display images in a (modern) terminal using RGB ANSI codes and unicode block graphics characters
Other
1.56k stars 111 forks source link

Add github workflow, Add CMake, Add dockcross #107

Closed bensuperpc closed 2 years ago

bensuperpc commented 3 years ago

How can we test this? Step by step instructions please.

With CMake + Ninja:

  1. cmake -S . -B build -G Ninja && ninja -C

    With CMake only build:

  2. cmake -S . -B build && cmake --build build

    With Dockcross (CMake and Ninja in docker):

    For linux aarch64

  3. ./tools/dockcross-cmake-builder.sh linux-arm64 -DCMAKE_BUILD_TYPE=Release

    For android aarch64

  4. ./tools/dockcross-cmake-builder.sh android-arm64 -DCMAKE_BUILD_TYPE=Release

    For linux x86_64 with clang

  5. ./tools/dockcross-cmake-builder.sh linux-x64-clang -DCMAKE_BUILD_TYPE=Release

    For Windows ARMv7

  6. ./tools/dockcross-cmake-builder.sh windows-armv7 -DCMAKE_BUILD_TYPE=Release

What happened before?

No CI and tests every push and merge

What should happen with this fix?

Improve code quality

Anything else we should know about this patch?

More info about dockcross (cross-platform compiler)

aaronliu0130 commented 3 years ago

Honestly I never liked the stale bot. The owner of this repo doesn't have enough free time to respond to the pulls and issues in time, so they just get closed. Another thing I've seen is that you add tests for A TON of linux distros and a set for Windows ARM without macOS or Windows x64. I'm not that into github actions, but it also seems like those are only names and you simply run them all on Ubuntu. Correct me if I'm wrong, but this is extremely concerning.

stefanhaustein commented 2 years ago

Original author doesn't seem responsive and it looks like overkill for this small project.

I think we should close this for now.

bensuperpc commented 2 years ago

Original author doesn't seem responsive and it looks like overkill for this small project.

I think we should close this for now.

I'm sorry, I didn't see the notification, I agree, it's a bit overkill, I will simplify the PR