Skip to content

Conversation

@printminion-co
Copy link
Contributor

No description provided.

@printminion-co printminion-co force-pushed the mk/dev/verbose_make_file branch from 9d33492 to 12e129b Compare July 11, 2025 16:16
…tions for better error handling and output clarity

in order to differentiate between error and fatal error

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…proved scope clarity

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…teps in configure.sh for improved clarity

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…on to prevent errors

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…app function for consistency

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…tion for consistency

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
…eport failed app enables

do not exit on first app enable failure.
output cumulated app enabling errors and exit

Signed-off-by: Misha M.-Kupriyanov <[email protected]>
@printminion-co printminion-co force-pushed the mk/dev/verbose_make_file branch from 12e129b to 6359d8b Compare July 11, 2025 16:56
@printminion-co printminion-co requested a review from Copilot July 11, 2025 16:57

This comment was marked as outdated.

@printminion-co printminion-co changed the title Mk/dev/verbose make file verbose make file Jul 11, 2025
@printminion-co printminion-co changed the title verbose make file verbose configuration files Jul 14, 2025
@printminion-co printminion-co requested a review from Copilot July 14, 2025 07:33

This comment was marked as outdated.

@printminion-co printminion-co requested a review from Copilot July 14, 2025 08:25

This comment was marked as outdated.

@printminion-co printminion-co force-pushed the mk/dev/verbose_make_file branch from 14ecedf to 2c6df09 Compare July 14, 2025 08:30
@printminion-co printminion-co requested a review from Copilot July 14, 2025 09:06
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors several shell scripts to introduce consistent logging, modularize configuration steps, and improve error handling with colorized output.

  • Introduced log_* and execute_occ_command utility functions for unified logging and OCC calls.
  • Broke monolithic scripts into smaller, well-named functions for each configuration task.
  • Updated apps-enable/disable scripts to track failures and provide clearer fatal error messages.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
readme.md Removed an extraneous blank line.
configure.sh Added logging functions, modularized OCC commands, and improved error handling.
configure-user-oidc.sh Replaced fail with log_fatal and added usage comments.
configure-object-store.sh Replaced fail with log_fatal and standardized error messages.
apps-enable.sh Renamed ooc to execute_occ_command, added failure tracking, unified logging.
apps-disable.sh Replaced fail with log_fatal for consistent fatal error handling.

@@ -1,185 +1,302 @@
#!/bin/sh
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

Consider adding set -euo pipefail after the shebang to ensure the script exits on errors and catches unset variables early.

Copilot uses AI. Check for mistakes.
# Check if required dependencies are available
# Usage: check_dependencies
check_dependencies() {
if ! which php >/dev/null 2>&1; then
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

Using which can be non‐portable; consider command -v php >/dev/null 2>&1 for POSIX compliance.

Suggested change
if ! which php >/dev/null 2>&1; then
if ! command -v php >/dev/null 2>&1; then

Copilot uses AI. Check for mistakes.
# Usage: verify_nextcloud_installation
verify_nextcloud_installation() {
log_info "Verifying HiDrive Next installation status..."
_main_status="$( execute_occ_command status 2>/dev/null | grep 'installed: ' | sed -r 's/^.*installed: (.+)$/\1/' )"
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

sed -r is not portable on macOS (BSD sed); use sed -E for extended regex support.

Suggested change
_main_status="$( execute_occ_command status 2>/dev/null | grep 'installed: ' | sed -r 's/^.*installed: (.+)$/\1/' )"
_main_status="$( execute_occ_command status 2>/dev/null | grep 'installed: ' | sed -E 's/^.*installed: (.+)$/\1/' )"

Copilot uses AI. Check for mistakes.
if ! execute_occ_command app:enable "${app_name}"
then
fail "Enabling app \"${app_name}\" failed."
echo "ERROR: Enabling app \"${app_name}\" failed."
Copy link

Copilot AI Jul 14, 2025

Choose a reason for hiding this comment

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

[nitpick] This error branch uses a plain echo while other failures use log_fatal; consider unifying to a consistent logging function for maintainability.

Suggested change
echo "ERROR: Enabling app \"${app_name}\" failed."
log_error "Enabling app \"${app_name}\" failed."

Copilot uses AI. Check for mistakes.
@printminion-co printminion-co merged commit 872e7aa into master Jul 14, 2025
7 checks passed
@printminion-co printminion-co deleted the mk/dev/verbose_make_file branch July 14, 2025 12:59
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.

3 participants