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

New Script: Automatic Ripping Machine #2165

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

thomashondema
Copy link
Contributor

@thomashondema thomashondema commented Feb 8, 2025

✍️ Description


  • Related Issue: #
  • Related PR: #
  • Related Discussion: #

✅ Prerequisites

The following steps must be completed for the pull request to be considered:

  • ✅ Self-review performed (I have reviewed my code to ensure it follows established patterns and conventions.)
  • ⏱️ Testing performed (I have thoroughly tested my changes and verified expected functionality.)

🛠️ Type of Change

Please check the relevant options:

  • ❌ Bug fix (non-breaking change that resolves an issue)
  • ❌ New feature (non-breaking change that adds functionality)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change unexpectedly)
  • ✅ New script (a fully functional and thoroughly tested script or set of scripts)

📋 Additional Information (optional)

Provide any extra context or screenshots about the feature or fix here.

@github-actions github-actions bot added new script A change that adds a new script website A change to the website labels Feb 8, 2025
@thomashondema thomashondema changed the title Add arm scripts and config New Script: Automatic Ripping Machine Feb 8, 2025
Comment on lines +35 to +38
msg_info "Updating ${APP}"
cd /opt/arm
git pull &>/dev/null
msg_ok "Updated ${APP}"

Choose a reason for hiding this comment

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

This needs to be reworked to not use git pull. Instead download the latest tarball.

Comment on lines +17 to +32
$STD apt-get install -y \
curl \
sudo \
mc \
git \
python3-pip \
python3-venv \
ffmpeg \
libdvdcss2 \
handbrake-cli \
makemkv-bin \
libavcodec-extra \
abcde \
flac \
imagemagick \
udev

Choose a reason for hiding this comment

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

Suggested change
$STD apt-get install -y \
curl \
sudo \
mc \
git \
python3-pip \
python3-venv \
ffmpeg \
libdvdcss2 \
handbrake-cli \
makemkv-bin \
libavcodec-extra \
abcde \
flac \
imagemagick \
udev
$STD apt-get install -y \
curl \
sudo \
mc \
git \
python3-pip \
python3-venv \
ffmpeg \
libdvdcss2 \
handbrake-cli \
makemkv-bin \
libavcodec-extra \
abcde \
flac \
imagemagick \
udev

Comment on lines +85 to +86
systemctl enable -q --now arm.service
systemctl enable -q --now armui.service

Choose a reason for hiding this comment

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

Suggested change
systemctl enable -q --now arm.service
systemctl enable -q --now armui.service
systemctl enable -q --now arm
systemctl enable -q --now armui

Comment on lines +14 to +15
var_os="ubuntu"
var_version="22.04"

Choose a reason for hiding this comment

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

Can you change it do debian 12, as this is our standard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I followed the documentation, so I used Ubuntu as recommended. However, it seems the most recent activity is aimed at Debian 12 support.

I will start from scratch based on this new information.

Copy link
Member

Choose a reason for hiding this comment

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

Ubuntu is fine too, but maybe 24.04?

# License: MIT | https://github.com/community-scripts/ProxmoxVE/raw/main/LICENSE
# Source: https://github.com/automatic-ripping-machine/automatic-ripping-machine

# App Default Values
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment

var_version="22.04"
var_unprivileged="0"

# App Output & Base Settings
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment

header_info "$APP"
base_settings

# Core
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment

Comment on lines +30 to +31
"username": "admin",
"password": "password"
Copy link
Member

Choose a reason for hiding this comment

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

Can these be randomly generated or its hardcoded to admin/password?

Copy link
Contributor

⚠️ The script ct/automatic-ripping-machine.sh failed with the following message:

./ct/automatic-ripping-machine.sh: line 2: .github/workflows/scripts/app-test/pr-build.func: No such file or directory
./ct/automatic-ripping-machine.sh: line 19: header_info: command not found
./ct/automatic-ripping-machine.sh: line 20: base_settings: command not found
./ct/automatic-ripping-machine.sh: line 23: variables: command not found
./ct/automatic-ripping-machine.sh: line 24: color: command not found
./ct/automatic-ripping-machine.sh: line 25: catch_errors: command not found
./ct/automatic-ripping-machine.sh: line 42: start: command not found
./ct/automatic-ripping-machine.sh: line 43: build_container: command not found
./ct/automatic-ripping-machine.sh: line 44: description: command not found
./ct/automatic-ripping-machine.sh: line 46: msg_ok: command not found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new script A change that adds a new script website A change to the website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants