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

Cannot remove settings once set #50

Open
electrofelix opened this issue Feb 22, 2017 · 2 comments
Open

Cannot remove settings once set #50

electrofelix opened this issue Feb 22, 2017 · 2 comments

Comments

@electrofelix
Copy link
Contributor

With the current variable testing before actions, it is not possible to unset or revert to default behaviour for any setting once previously set.

I wonder if it would be better to follow this pattern:

set_gerrit_config() {
  su-exec ${GERRIT_USER} git config -f "${GERRIT_SITE}/etc/gerrit.config" "$@"
}

unset_gerrit_config() {
  su-exec ${GERRIT_USER} git config -f "${GERRIT_SITE}/etc/gerrit.config" --unset "$@"
}

# test whether the variable has been set, even if blank
# if set and empty, remove the gerrit setting
# if set and non-empty, set the gerrit config option
[ "${TEST_VAR+x}" = "x" ] && if [ -z "${TEST_VAR}" ]
then
  unset_git_config some.gerrit_setting
else
  set_git_config some.gerrit_setting ${TEST_VAR}
fi

Thoughts?

This would certainly work from a docker-compose environment and I believe should avoid removing any settings unless the variable is explicitly exported while empty.

@thinkernel
Copy link
Contributor

Hi there. I see the point that you need a way to unset some properties on startup. I agree with you that it can be a good feature.
Unfortunately, the proposal that provided by you will make just one line logic into about six lines. That means gerrit-entrypoint.sh will be about 6 times longer and most of those lines are if/else.
I think we might have to figure out another simpler way to accomplish this.

@electrofelix
Copy link
Contributor Author

I'm hoping I can turn it into a function that is passed the name of the variable to test and corresponding config setting, but assuming that's possible, can you foresee and unexpected side effects from using the test for set but empty to remove settings?

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

No branches or pull requests

2 participants