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

Update README #8

Merged
merged 4 commits into from
Apr 19, 2023
Merged

Conversation

tyler36
Copy link
Contributor

@tyler36 tyler36 commented Apr 12, 2023

This PR makes some minor updates to the readme.

  • Fix some typos
  • Adds links to referenced sites

Fixes #7

@tyler36
Copy link
Contributor Author

tyler36 commented Apr 12, 2023

The README also has a section called "Disabling MySQL & MariaSQL"

This addon automatically disables these in its config.sqlsrv.yaml so I think we can remove this section or perhaps rework?

@robertoperuzzo
Copy link
Collaborator

robertoperuzzo commented Apr 12, 2023

I think we could leave some info in the readme about the config.*.yml because someone (like me) may use this add-on to connect to a second external sqlsrv db and not as main Drupal db. What do you think?

@tyler36
Copy link
Contributor Author

tyler36 commented Apr 12, 2023

@robertoperuzzo

Sounds reasonable. I think it's confusing that the addon describes how to disable the default database but disables it via a different file.

Perhaps we need something like:

"This addons disables the default database with via the config.sqlsrv.yaml"

And move the current instructions into a "Manually disabling the default DDEV database" for people who prefer to manage things themselves.

Does this make sense or am I overthinking things?

@tyler36 tyler36 mentioned this pull request Apr 14, 2023
@robertoperuzzo
Copy link
Collaborator

@robertoperuzzo

Sounds reasonable. I think it's confusing that the addon describes how to disable the default database but disables it via a different file.

Perhaps we need something like:

"This addons disables the default database with via the config.sqlsrv.yaml"

And move the current instructions into a "Manually disabling the default DDEV database" for people who prefer to manage things themselves.

Does this make sense or am I overthinking things?

It makes sense.

We could replace the paragraph like that:

Manually enabling MySQL/MariaDB

This addons disables the default database by automatically adding omit_containers: [db,dba] in the config.sqlsrv.yaml

If your project needs to use both MariaDB and MS SQL Server databases, you have to remove #ddev-generated and
omit_containers: [db,dba] from config.sqlsrv.yaml.

See .ddev/config.yaml Options for additional notes.

@tyler36
Copy link
Contributor Author

tyler36 commented Apr 18, 2023

That looks better to me.

Not sure if you want to update this PR or open a new one.

README.md Outdated
@@ -20,7 +20,7 @@ ddev get robertoperuzzo/ddev-sqlsrv
ddev restart
```

If in your project you already have a `.ddev/.env` file, you need to add the following lines to it:
If your project you already has a `.ddev/.env` file, you need to add the following lines to it:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I hint to replace it with => "If your project already has a custom .ddev/.env file, you need to add the following lines to it:"

@robertoperuzzo robertoperuzzo merged commit 80457e5 into ddev:main Apr 19, 2023
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.

📓 update readme
2 participants