Skip to content

feat: preserve dir structure for custom recipes#383

Merged
canercidam merged 4 commits intomainfrom
fryd/custom-recipes-subdir-support
Feb 25, 2026
Merged

feat: preserve dir structure for custom recipes#383
canercidam merged 4 commits intomainfrom
fryd/custom-recipes-subdir-support

Conversation

@fkondej
Copy link
Contributor

@fkondej fkondej commented Feb 25, 2026

No description provided.

@fkondej fkondej self-assigned this Feb 25, 2026
@fkondej fkondej requested a review from canercidam as a code owner February 25, 2026 09:15
@claude
Copy link
Contributor

claude bot commented Feb 25, 2026

Claude finished @fkondej's task in 2m 10s —— View job


PR Review: feat: preserve dir structure for custom recipes

  • Gather context and read PR diff
  • Review code changes in playground/custom_recipes.go
  • Review test changes in playground/custom_recipes_test.go
  • Review custom recipe file changes
  • Post review feedback

Overall this is a clean change — preserving subdirectory structure and applying correct file permissions are both solid improvements. A few issues to address:

YAML filter skips subdirectory YAML files

Both GenerateCustomRecipeToDir (line 237) and listCustomRecipeFiles (line 352) use the same filter:

if (strings.HasSuffix(fileName, ".yaml") || strings.HasSuffix(fileName, ".yml")) && fileName != yamlFile {
    return nil
}

This skips any YAML/YML file that isn't playground.yaml, regardless of where it sits in the directory tree. Now that subdirectories are preserved, a file like config/settings.yaml would be silently dropped. The filter should be scoped to root-level files only, e.g.:

if relPath == fileName && (strings.HasSuffix(fileName, ".yaml") || ...) && fileName != yamlFile {

See inline comment for details.

Missing trailing newline

custom-recipes/buildernet/mkosi/scripts/prepare.sh is missing its trailing newline. See inline comment.

Minor observations

  • Deleted send-tx.sh: This file was removed but isn't mentioned in the commit messages. If intentional, that's fine, but worth confirming it's not needed elsewhere.
  • Test coverage: The new tests for directory preservation and permission checks are well-structured.

@claude
Copy link
Contributor

claude bot commented Feb 25, 2026

Claude finished @fkondej's task in 59s —— View job


PR Review: feat: preserve dir structure for custom recipes

  • Gather context and read PR diff
  • Review code changes in playground/custom_recipes.go
  • Review test changes in playground/custom_recipes_test.go
  • Review custom recipe file changes
  • Check if previous review issues were addressed
  • Post review feedback

The previous review's two main issues have both been addressed:

  1. YAML filter scoped to root-level — Fixed. Both GenerateCustomRecipeToDir (line 237) and listCustomRecipeFiles (line 352) now correctly use relPath == fileName to only skip root-level YAML files, allowing subdirectory YAML files to pass through.

  2. Missing trailing newline in prepare.sh — Fixed. The file now ends with a newline.

Remaining observations

The code changes look clean. Subdirectory preservation via filepath.Rel + os.MkdirAll is straightforward, and the permission logic (0755 for .sh, 0644 for everything else) is a good improvement over blanket 0755. Test coverage for both directory structure and permissions is solid.

No new issues found — LGTM.


@canercidam canercidam merged commit 399b5af into main Feb 25, 2026
7 checks passed
@canercidam canercidam deleted the fryd/custom-recipes-subdir-support branch February 25, 2026 17:15
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