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

Issue #228: Fix remote column missing in schema check #4

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

jnlar
Copy link

@jnlar jnlar commented Dec 20, 2022

Upstream issue: jleyva#228

Version bumping the remote column update as well as the plugin version in version.php

Copy link

@keevan keevan 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 good. Some indentation tweaks and a version change required.

In terms of impact, this shouldn't impact existing sites without the missing remote column issue, as it will skip adding the field if it already exists. This upgrade is to fix sites which were in a bad state.

db/upgrade.php Outdated Show resolved Hide resolved
version.php Outdated Show resolved Hide resolved
@jnlar jnlar force-pushed the issue228-column-remote-missing branch from b72c284 to d669852 Compare December 20, 2022 02:53
@jnlar jnlar marked this pull request as ready for review December 20, 2022 02:55
Copy link

@keevan keevan left a comment

Choose a reason for hiding this comment

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

2 small changes, but I think we're in a good spot after that.

Thanks Jonathan

db/upgrade.php Outdated Show resolved Hide resolved
db/upgrade.php Outdated Show resolved Hide resolved
@jnlar jnlar force-pushed the issue228-column-remote-missing branch from d669852 to 28ad15b Compare December 20, 2022 03:08
@jnlar jnlar force-pushed the issue228-column-remote-missing branch from 28ad15b to 0cae6df Compare December 20, 2022 03:28
@keevan keevan merged commit 07f258d into MOODLE_36_STABLE-catalyst Dec 20, 2022
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.

2 participants