Skip to content
This repository was archived by the owner on Sep 26, 2025. It is now read-only.

feat: deprecate data-folder and expose branch flags#966

Merged
dbarrosop merged 1 commit into
mainfrom
branch
Jun 9, 2025
Merged

feat: deprecate data-folder and expose branch flags#966
dbarrosop merged 1 commit into
mainfrom
branch

Conversation

@dbarrosop
Copy link
Copy Markdown
Member

@dbarrosop dbarrosop commented Jun 5, 2025

User description

The data-folder flag is basically useless (and potentially misleading) as we don't store data in any folder anymore.


PR Type

Enhancement


Description

• Remove deprecated data-folder flag and related code
• Expose branch flag to users with detailed usage description
• Simplify path structure by removing unused data folder parameter
• Update docker compose to use computed data folder path


Changes walkthrough 📝

Relevant files
Enhancement
clienv.go
Remove data folder from CLI environment                                   

clienv/clienv.go

• Remove flagDataFolder parameter from NewPathStructure call

+0/-1     
filesystem.go
Simplify path structure by removing data folder                   

clienv/filesystem.go

• Remove dataFolder field from PathStructure struct
• Remove
dataFolder parameter from constructor
• Remove DataFolder() method

+1/-7     
flags.go
Remove data folder flag and expose branch flag                     

clienv/flags.go

• Remove flagDataFolder constant and related flag definition
• Expose
branch flag by setting Hidden: false
• Add detailed usage description
for branch flag
• Remove unused dataFolder variable

+3/-12   
up.go
Remove data folder parameter from compose setup                   

cmd/dev/up.go

• Remove ce.Path.DataFolder() parameter from ComposeFileFromConfig
call

+0/-1     
compose.go
Compute data folder path internally in compose                     

dockercompose/compose.go

• Remove dataFolder parameter from function signatures
• Compute data
folder path internally using dotNhostFolder
• Update function calls to
remove data folder parameter

+1/-3     
Tests
validate_test.go
Update test to match simplified path structure                     

cmd/config/validate_test.go

• Remove dataFolder parameter from NewPathStructure call in test

+0/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown
    Contributor

    github-actions Bot commented Jun 5, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Path Construction

    The data folder path is now computed as filepath.Join(dotNhostFolder, "data") instead of using the branch-specific path that was previously passed as parameter. This changes the data persistence behavior and should be validated to ensure it doesn't break existing functionality.

    dataFolder := filepath.Join(dotNhostFolder, "data")
    postgres, err := postgres(cfg, subdomain, postgresPort, dataFolder, pgVolumeName)
    Flag Exposure

    The branch flag is now exposed to users (Hidden: false) with a detailed usage description. The long usage text and behavior change should be validated to ensure it provides clear guidance without overwhelming users.

    Usage:   "Git branch name. If not set, it will be detected from the current git repository. This flag is used to dynamically create docker volumes for each branch. If you want to have a static volume name or if you are not using git, set this flag to a static value.", //nolint:lll
    EnvVars: []string{"BRANCH"},
    Value:   branch,
    Hidden:  false,

    @github-actions
    Copy link
    Copy Markdown
    Contributor

    github-actions Bot commented Jun 5, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add branch isolation to data folder

    The dataFolder is now hardcoded without branch-specific isolation. This could cause
    data conflicts when switching between branches since all branches will share the
    same data directory.

    dockercompose/compose.go [537-538]

     pgVolumeName := "pgdata_" + sanitizeBranch(branch)
    -dataFolder := filepath.Join(dotNhostFolder, "data")
    +dataFolder := filepath.Join(dotNhostFolder, "data", sanitizeBranch(branch))
     postgres, err := postgres(cfg, subdomain, postgresPort, dataFolder, pgVolumeName)
    Suggestion importance[1-10]: 8

    __

    Why: This is a critical issue that could cause data conflicts between branches. The suggestion correctly identifies that removing branch-specific isolation from the dataFolder path could lead to data corruption when switching branches, and provides the proper fix by adding sanitizeBranch(branch) to maintain isolation.

    Medium

    @dbarrosop dbarrosop merged commit a46a4b6 into main Jun 9, 2025
    10 of 20 checks passed
    @dbarrosop dbarrosop deleted the branch branch June 9, 2025 10:42
    Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants