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.

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

switched to centos stream 8 image, added building of aardvark-dns, up… #3

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thimslugga
Copy link

refactored some code, updated podman and dependencies

Copy link
Owner

@popsUlfr popsUlfr left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to check out the pull request. Great work!

There seem to be unneeded files that could be removed.
The .gitignore has a lot of generic rules.
If the crun.static binary has been renamed then it should also patch the new name in the config file from the entrypoint.sh.

mkdir -p "$pkgdir/usr/bin"
cp crun.static "$pkgdir/usr/bin/"
cp "$pkgname-static" "$pkgdir/usr/bin/crun-static"
Copy link
Owner

Choose a reason for hiding this comment

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

You've renamed crun.static to crun-static here but it's still patched as crun.static in the config file in the entrypoint.sh.

# Windows shortcuts
*.lnk

build.log
Copy link
Owner

Choose a reason for hiding this comment

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

There are too many generic rules in the .gitignore it should be kept to a minimum in my opinion.

@@ -0,0 +1,7 @@
FROM quay.io/centos/centos:stream8
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this file is needed to build the project ?

```sh
ln -s podman-*.AppImage podman
```
And invoke `podman` directly:

Noww invoke `podman` directly:
Copy link
Owner

Choose a reason for hiding this comment

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

Typo Now.

@@ -0,0 +1,72 @@
autoconf
Copy link
Owner

Choose a reason for hiding this comment

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

This is linked to the Containerfile I guess, I think this is not needed to build the project too ?

Also the justfile seems to be an oversight ?

#!/usr/bin/env bash
#set -eux

if [ $# -ne 1 ] || [[ "$1" == "-h" ]]; then
Copy link
Owner

Choose a reason for hiding this comment

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

Might as well use [[ ]] for both tests ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants