Skip to content

Conversation

@CLHatch
Copy link
Contributor

@CLHatch CLHatch commented Jan 27, 2026

Pull request

Purpose
Describe the problem or feature in addition to a link to the issues.

Approach
How does this change address the problem?

Open Questions and Pre-Merge TODOs
Check all boxes as they are completed

  • Use github checklists. When solved, check the box and explain the answer.

Learning
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem

Requirements
Check all boxes as they are completed

Summary by Sourcery

Reorganize shell script folders and application assets while adding a migration path for existing user data.

Enhancements:

  • Standardize includes and scripts directory names by renaming hidden '.includes' and '.scripts' folders to 'includes' and 'scripts'.
  • Introduce an application assets folder and update defaults, themes, and related paths to use it instead of dot-prefixed directories.
  • Change log file naming and locations so fatal logs are stored alongside the main application log with consistent filenames.
  • Add a migration helper to move legacy log files and state subfolders to the new folder structure at runtime.

CI:

  • Update labeler and test workflow patterns to track and execute scripts from the new 'scripts' directory instead of '.scripts'.

@CLHatch CLHatch requested review from a team as code owners January 27, 2026 19:08
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Jan 27, 2026

Reviewer's Guide

Refactors the project’s directory layout (notably removing leading dots from includes/scripts and standardizing asset/state locations) while adding a migration helper to preserve existing user data and updating references, logging paths, and CI config accordingly.

Sequence diagram for startup sourcing and migration process

sequenceDiagram
    actor User
    participant Shell
    participant main_sh as main.sh
    participant misc as misc_functions.sh
    participant globals as global_variables.sh
    participant migrate as migration_functions.sh

    User ->> Shell: run ds / main.sh
    Shell ->> main_sh: execute

    main_sh ->> misc: source includes/misc_functions.sh
    misc -->> main_sh: functions loaded

    main_sh ->> globals: source includes/global_variables.sh
    globals -->> main_sh: paths and folders configured

    main_sh ->> migrate: source includes/migration_functions.sh
    migrate -->> main_sh: MigrateFilesAndFolders available

    main_sh ->> main_sh: check MigrateFilesAndFolders is declared
    alt migration helper available
        main_sh ->> migrate: MigrateFilesAndFolders()
        migrate ->> migrate: build MigrationFileMap and MigrationFolderMap
        loop for each mapped file
            migrate ->> migrate: if NewFile missing and OldFile exists
            migrate ->> migrate: RunAndLog mv OldFile -> NewFile
        end
        loop for each mapped folder
            migrate ->> migrate: if NewFolder missing and OldFolder exists
            migrate ->> migrate: RunAndLog mv OldFolder -> NewFolder
        end
    else migration helper missing
        main_sh ->> main_sh: continue without migration
    end

    main_sh ->> main_sh: source remaining includes (pm, run_script, dialog, ds, test, usage, cmdline)
    main_sh -->> Shell: ready to handle commands
    Shell -->> User: DockSTARTer ready
Loading

Flow diagram for file and folder migration and new layout

flowchart LR
    subgraph OldLayout["Old layout (before refactor)"]
        of1["${SCRIPTPATH}/dockstarter.log"]
        of2["${APPLICATION_STATE_FOLDER}/fatal.log"]
        of3["${SCRIPTPATH}/fatal.log"]

        od1["${APPLICATION_STATE_FOLDER}/.theme"]
        od2["${APPLICATION_STATE_FOLDER}/.timestamps"]
        od3["${APPLICATION_STATE_FOLDER}/.instances"]

        oinc["${SCRIPTPATH}/.includes"]
        oscr["${SCRIPTPATH}/.scripts"]

        odef["${SCRIPTPATH}/.defaults"]
        othm["${SCRIPTPATH}/.themes"]
    end

    subgraph NewLayout["New layout (after refactor)"]
        subgraph LogsState["State and log files"]
            nf1["${APPLICATION_LOG} = ${XDG_STATE_HOME}/${APPLICATION_NAME,,}/${APPLICATION_NAME,,}.log"]
            nf2["${FATAL_LOG} = ${XDG_STATE_HOME}/${APPLICATION_NAME,,}/${APPLICATION_NAME,,}.fatal.log"]

            nd2["${TIMESTAMPS_FOLDER}"]
            nd3["${INSTANCES_FOLDER}"]
        end

        subgraph Assets["Assets in repo"]
            aas["${APPLICATION_ASSETS_FOLDER} = ${SCRIPTPATH}/assets"]
            ndef["${DEFAULTS_FOLDER} = ${APPLICATION_ASSETS_FOLDER}/defaults"]
            nthm["${THEME_FOLDER} = ${APPLICATION_ASSETS_FOLDER}/themes"]
        end

        subgraph IncludesScripts["Code layout"]
            ninc["${SCRIPTPATH}/includes"]
            nscr["${SCRIPTPATH}/scripts"]
        end
    end

    subgraph Migration["Migration helper"]
        mh["MigrateFilesAndFolders() in includes/migration_functions.sh"]
    end

    of1 -->|mapped by MigrationFileMap| mh
    of2 -->|mapped by MigrationFileMap| mh
    of3 -->|mapped by MigrationFileMap| mh

    od1 -->|mapped by MigrationFolderMap| mh
    od2 -->|mapped by MigrationFolderMap| mh
    od3 -->|mapped by MigrationFolderMap| mh

    mh --> nf1
    mh --> nf2
    mh --> nd2
    mh --> nd3

    oinc --> ninc
    oscr --> nscr

    odef --> aas
    othm --> aas

    classDef old fill:#fff5f5,stroke:#ff7f7f,stroke-width:1px
    classDef new fill:#f5fff5,stroke:#7fff7f,stroke-width:1px

    class of1,of2,of3,od1,od2,od3,oinc,oscr,odef,othm old
    class nf1,nf2,nd2,nd3,aas,ndef,nthm,ninc,nscr new
Loading

File-Level Changes

Change Details Files
Standardize log file locations and introduce fatal log naming convention.
  • Set APPLICATION_LOG and FATAL_LOG to live under XDG_STATE_HOME in an application-specific subfolder
  • Rename fatal log to use an application-specific filename pattern (*.fatal.log)
  • Remove legacy in-place log migration logic from main.sh in favor of centralized migration handling
main.sh
Move .includes to includes and wire in a migration step during startup.
  • Replace all .includes/* source paths with includes/* in main.sh
  • Introduce sourcing of a new migration_functions.sh file and invoke MigrateFilesAndFolders when available
  • Adjust repo layout validation in check_repo to look for includes and scripts directories instead of .includes and .scripts
main.sh
Re-home defaults/themes/templates/instances/timestamps/temp folders under new non-dotted asset/state directories.
  • Introduce APPLICATION_ASSETS_FOLDER pointing at ${SCRIPTPATH}/assets
  • Change DEFAULTS_FOLDER_NAME, THEME_FOLDER_NAME, INSTANCES_FOLDER_NAME, TIMESTAMPS_FOLDER_NAME, TEMP_FOLDER_NAME to non-dotted names
  • Point DEFAULTS_FOLDER and THEME_FOLDER to subfolders of APPLICATION_ASSETS_FOLDER instead of directly under SCRIPTPATH
includes/global_variables.sh
Move .scripts to scripts and update all script-dispatch helpers and tests.
  • Update run_script/check_script helpers to resolve scripts from ${SCRIPTPATH}/scripts instead of .scripts
  • Adjust test helper to load scripts from the new scripts folder
  • Update README to reflect the scripts folder name change
includes/run_script.sh
includes/test_functions.sh
scripts/README.md
scripts/app_instance_file.sh
scripts/app_instance_folder.sh
Update CI and labeler configuration to track scripts in the new folder.
  • Change labeler patterns from .scripts/* to scripts/
  • Modify tests workflow to discover scripts from scripts/.sh and adjust sed expression accordingly
.github/labeler.yml
.github/workflows/tests.yml
Add a migration utility to move legacy files/folders into the new structure on startup.
  • Introduce migration_functions.sh with MigrateFilesAndFolders implementing file and folder move mappings
  • Migrate legacy log files (dockstarter.log, fatal.log) to new APPLICATION_LOG/FATAL_LOG locations when missing
  • Migrate legacy theme, timestamps, and instances folders from dotted paths under APPLICATION_STATE_FOLDER into the new configured folders via RunAndLog and warn messaging
includes/migration_functions.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@github-actions github-actions bot added ci Automatic label repo Automatic label labels Jan 27, 2026
@CLHatch CLHatch merged commit 04719e2 into main Jan 27, 2026
16 checks passed
@CLHatch CLHatch deleted the FolderStructure branch January 27, 2026 19:09
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `includes/migration_functions.sh:21-23` </location>
<code_context>
+	for OldFile in "${!MigrationFileMap[@]}"; do
+		local NewFile="${MigrationFileMap[${OldFile}]}"
+
+		if [[ ! -f ${NewFile} && -f ${OldFile} ]]; then
+			warn "Migrating '${C["File"]}${OldFile}${NC}' to '${C["File"]}${NewFile}${NC}'."
+			RunAndLog warn "mv:warn" \
+				warn "Failed to migrate '${C["File"]}${OldFile}${NC}'." \
+				mv "${OldFile}" "${NewFile}" || true
</code_context>

<issue_to_address>
**issue (bug_risk):** Using `mv ... || true` inside `RunAndLog` suppresses failures and prevents the error branch from running.

Because the `mv` command is wrapped with `|| true`, the overall command always returns 0 to `RunAndLog`, so the failure message branch is never triggered even if `mv` fails. To keep migration failures visible while still preventing the script from aborting, remove `|| true` and, if necessary, handle non‑zero statuses via `RunAndLog`’s return value instead.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +21 to +23
if [[ ! -f ${NewFile} && -f ${OldFile} ]]; then
warn "Migrating '${C["File"]}${OldFile}${NC}' to '${C["File"]}${NewFile}${NC}'."
RunAndLog warn "mv:warn" \
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Using mv ... || true inside RunAndLog suppresses failures and prevents the error branch from running.

Because the mv command is wrapped with || true, the overall command always returns 0 to RunAndLog, so the failure message branch is never triggered even if mv fails. To keep migration failures visible while still preventing the script from aborting, remove || true and, if necessary, handle non‑zero statuses via RunAndLog’s return value instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Automatic label repo Automatic label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants