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

fix: add more checks for existing approot #6208

Merged
merged 1 commit into from
May 20, 2024

Conversation

stasadev
Copy link
Member

The Issue

app.CheckExistingAppInApproot() does not work properly if you run ddev config several times and change the project name:

$ ddev config --project-name=d10
Creating a new DDEV project config in the current directory (/home/stas/code/ddev/d10) 
Once completed, your configuration will be written to /home/stas/code/ddev/d10/.ddev/config.yaml
 
Configuring a 'php' project with docroot '' at /home/stas/code/ddev/d10 
Configuration complete. You may now run 'ddev start'. 

$ ddev config --project-name=d10-rename
You are reconfiguring the project at /home/stas/code/ddev/d10.
The existing configuration will be updated and replaced.
Configuring a 'php' project with docroot '' at /home/stas/code/ddev/d10
Configuration complete. You may now run 'ddev start'.

$ ddev config --project-name=d10-rename-2
this project root /home/stas/code/ddev/d10 already contains a project named d10. You may want to remove the existing project with "ddev stop --unlist d10"

The ~/.ddev/project_list.yaml file:

d10:
    approot: /home/stas/code/ddev/d10
d10-rename:
    approot: /home/stas/code/ddev/d10

How This PR Solves The Issue

Adds app.CheckExistingAppInApproot() to more places.

Manual Testing Instructions

$ ddev config --project-name=d10

$ ddev config --project-name=d10-rename
You are reconfiguring the project at /home/stas/code/ddev/d10.
The existing configuration will be updated and replaced.
this project root /home/stas/code/ddev/d10 already contains a project named d10. You may want to remove the existing project with "ddev stop --unlist d10"

$ echo d10-rename | ddev config
You are reconfiguring the project at /home/stas/code/ddev/d10.
The existing configuration will be updated and replaced.
Project name (d10): this project root /home/stas/code/ddev/d10 already contains a project named d10. You may want to remove the existing project with "ddev stop --unlist d10"

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@stasadev stasadev requested a review from a team as a code owner May 17, 2024 17:15
Copy link

Copy link
Contributor

@bmartinez287 bmartinez287 left a comment

Choose a reason for hiding this comment

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

A safety check based around a consistent outcome seems like great idea.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Looks great to me, tests fine, thanks!

@rfay rfay merged commit 9dff0d5 into ddev:master May 20, 2024
18 checks passed
@stasadev stasadev deleted the 20240517_stasadev_check_for_approot branch May 21, 2024 12:07
jonesrussell pushed a commit to jonesrussell/ddev that referenced this pull request Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants