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

Remove checks for config management #25

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Yuvania
Copy link
Contributor

@Yuvania Yuvania commented Sep 22, 2023

This PR refers to this issue: #24

@Yuvania Yuvania self-assigned this Sep 22, 2023
Copy link
Contributor

@dalin- dalin- left a comment

Choose a reason for hiding this comment

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

You've done the first step here.

I totally regret writing this in BASH, because it's a language that values using fewer characters over clarity.

This first part

. "./some/directory"

in PHP would be

include_once("./some/directory");

I'm guessing that if you run this script with this change you'll get a fatal error.

So you also need to find further down where the function within that file is called and remove it.

Copy link
Contributor

@dalin- dalin- left a comment

Choose a reason for hiding this comment

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

Overall I think we're making this change in the wrong place. The function drupal8_check_config_management (in functions/drupal8/drupal8-check-config-management) does 3 things:

  • checks if the config module is installed (we want to remove this)
  • checks if config split module is installed (we want to remove this)
  • checks if config is overridden, if so gives some options about how to deal with it (we want to keep this).

So I think we only need to change that function, and not change the places that call that function.

Comment on lines -58 to -59

drupal8)
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work like you're hoping. In bash/sh/zsh a case is the same as a PHP switch. So since you removed drupal8) then it will fall back to *) and get the message below: "This script has no configuration management automation...".

Instead I think you want to just leave the drupal8) section empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dalin- I understand, I understand, I'll do that, thanks for explaining in more detail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants