Skip to content

starting a makefile (need upstream changes tho) #8

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

Open
SomethingGeneric wants to merge 4 commits into
base: Go
Choose a base branch
from
Open

starting a makefile (need upstream changes tho) #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e29cc98
starting a makefile (need upstream changes tho)
SomethingGeneric Feb 1, 2023
ae6b84f
Merge branch 'alchemicode:Go' into Go
SomethingGeneric Feb 7, 2023
09cbeb0
Update Makefile
SomethingGeneric Feb 7, 2023
907df79
chore: ensure Renovate automerge configuration
SomethingGeneric Feb 26, 2026
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
5 changes: 5 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Release bundles
*.zip
*.tar.gz

# If you prefer the allow list template instead of the deny list, see community template:
# https://github.com/github/gitignore/blob/main/community/Golang/Go.AllowList.gitignore
#
Expand All @@ -7,6 +11,7 @@
*.dll
*.so
*.dylib
Server/jServ

# Test binary, built with `go test -c`
*.test
Expand Down
14 changes: 14 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
build:
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Missing .PHONY declarations for the targets. Since build, release, and deploy are not actual files, they should be declared as .PHONY to ensure make always executes them even if files with those names exist. Add .PHONY: build release deploy at the top of the Makefile.

Copilot uses AI. Check for mistakes.
cd Server && go build
release: build
mkdir js-linux
cp Server/config.json js-linux/.
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The config.json file may contain sensitive configuration including IP addresses, ports, and permissions settings. Including it in release bundles could leak information about production deployments. Consider documenting that users should customize config.json after installation, or provide a template/example config instead of the actual config file.

Suggested change
cp Server/config.json js-linux/.
printf '{\n}\n' > js-linux/config.json

Copilot uses AI. Check for mistakes.
cp Server/jServ js-linux/.
tar -czvf jserv-linux-amd64.tar.gz js-linux/*
rm -rf js-linux
Comment on lines +4 to +8
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

There's a naming inconsistency: the temporary directory is named 'js-linux' but the output tarball is 'jserv-linux-amd64.tar.gz'. Consider using consistent naming, such as 'jserv-linux' for the temporary directory to match the tarball name pattern.

Suggested change
mkdir js-linux
cp Server/config.json js-linux/.
cp Server/jServ js-linux/.
tar -czvf jserv-linux-amd64.tar.gz js-linux/*
rm -rf js-linux
mkdir jserv-linux
cp Server/config.json jserv-linux/.
cp Server/jServ jserv-linux/.
tar -czvf jserv-linux-amd64.tar.gz jserv-linux/*
rm -rf jserv-linux

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +8
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The release target should check if the build succeeded before proceeding with packaging. If go build fails, the release target will continue and package a potentially non-existent or stale binary. Consider adding error handling or making the build dependency explicit by checking for the binary's existence.

Copilot uses AI. Check for mistakes.
deploy: build
[[ ! -d /etc/jserv ]] && mkdir /etc/jserv
[[ -f /usr/bin/jServ ]] && rm /usr/bin/jServ
Comment on lines +10 to +11
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The conditional commands will cause make to fail when the condition is false. In make, if any command returns a non-zero exit code, the target fails. When [[ ! -d /etc/jserv ]] evaluates to false (directory exists), the entire command returns false, causing make to stop. Use || true at the end of each line, or rewrite using shell if-statements wrapped in a single shell invocation.

Suggested change
[[ ! -d /etc/jserv ]] && mkdir /etc/jserv
[[ -f /usr/bin/jServ ]] && rm /usr/bin/jServ
if [ ! -d /etc/jserv ]; then mkdir /etc/jserv; fi
if [ -f /usr/bin/jServ ]; then rm /usr/bin/jServ; fi

Copilot uses AI. Check for mistakes.
cp Server/jServ /usr/bin/.
cp Server/config.json /etc/jserv/.
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Similar to the release target, copying config.json directly from Server/ to /etc/jserv/ will overwrite any existing production configuration. On subsequent deployments, this could reset customized settings. Consider either skipping the copy if the file exists, or providing clear documentation that users should backup their config before deploying.

Suggested change
cp Server/config.json /etc/jserv/.
[[ ! -f /etc/jserv/config.json ]] && cp Server/config.json /etc/jserv/.

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +13
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

The deploy target requires root privileges (writes to /usr/bin and /etc) but doesn't validate permissions or provide clear error messages. The target will fail with cryptic errors if not run with sudo. Consider adding a permission check at the start of the target, or documenting in comments that this target must be run with sudo.

Copilot uses AI. Check for mistakes.
# systemd unit needed
11 changes: 11 additions & 0 deletions renovate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": ["config:recommended"],
"automerge": true,
"packageRules": [
{
"matchUpdateTypes": ["minor", "patch", "digest"],
"automerge": true
}
]
Comment on lines +4 to +10
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

Setting automerge to true at both the root level (line 4) and in packageRules (line 8) is redundant. The root-level automerge enables it for all updates, while the packageRule only enables it for minor, patch, and digest updates. If the intention is to only automerge minor/patch/digest updates, remove the root-level automerge. If the intention is to automerge everything, remove the packageRule as it's redundant.

Suggested change
"automerge": true,
"packageRules": [
{
"matchUpdateTypes": ["minor", "patch", "digest"],
"automerge": true
}
]
"automerge": true

Copilot uses AI. Check for mistakes.
}