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

Initial move of server to seperate module. #206

Merged
merged 5 commits into from
Jul 25, 2022

Conversation

ekes
Copy link
Member

@ekes ekes commented Jul 11, 2022

Working on the same principle as localgovdrupal/localgov_search#34
Has the added bonus of assisting like #201

Would need. Tests altering.
I'm thinking that existing tests would require localgov_directories_db.
There would be an additional Functional Base Test than any server
modules can inherit to check integration (as
localgovdrupal/localgov_search#34)

Working on the same principle as localgovdrupal/localgov_search#34
Has the added bonus of assisting like #201

Would need. Tests altering.
I'm thinking that existing tests would require localgov_directories_db.
There would be an additional Functional Base Test than any server
modules can inherit to check integration (as
localgovdrupal/localgov_search#34)
@finnlewis
Copy link
Member

This goes with localgovdrupal/localgov_search#40

@finnlewis
Copy link
Member

From discussion in the search working group:

@ekes thinks it would be good to write an update path to enable the module but not install it's config!

@ekes will need to look at this before further testing.

It is 'as if' the _db module was used for installing configuration on
existing sites, so making that clear by enabling, but bypassing
configuration.

The description clarifies the module provides the initial server
configuration.
Copy link
Contributor

@Adnan-cds Adnan-cds left a comment

Choose a reason for hiding this comment

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

Looks mostly okay. I have added two minor suggestions.

It may be better to include in the README that the localgov_directories_db module needs to be enabled on a new site for any kind of Directory search to work.

Also, if any other search backend is later enabled and a module similar to localgov_directories_db is installed to support that backend, there could be a conflict over the server setting. So may be better to uninstall localgov_directories_db before switching to a new backend. The README can clarify all these.

@ekes ekes marked this pull request as ready for review July 25, 2022 10:08
@ekes
Copy link
Member Author

ekes commented Jul 25, 2022

It may be better to include in the README that the localgov_directories_db module needs to be enabled on a new site for any kind of Directory search to work.

Something like
31dcbee
?

Copy link
Contributor

@Adnan-cds Adnan-cds left a comment

Choose a reason for hiding this comment

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

Looks good now. Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done sprint 8
Development

Successfully merging this pull request may close these issues.

5 participants