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

Fix name of cache key column to be all lower case rather than camel c… #14359

Merged
merged 1 commit into from
May 28, 2019

Conversation

seamuslee001
Copy link
Contributor

…ase in civicrm_prevnext_cache

Overview

This renames the cacheKey column to be cachekey to be more standard with other column names

Before

camelCase column name used

After

lower case used

ping @eileenmcnaughton @colemanw

@civibot
Copy link

civibot bot commented May 27, 2019

(Standard links)

@civibot civibot bot added the master label May 27, 2019
@eileenmcnaughton
Copy link
Contributor

Looks good - I'll let jenkins weigh in before I dig deeply

@seamuslee001
Copy link
Contributor Author

Test failures appear to relate will investigate shortly

…ase in civicrm_prevnext_cache

Fix test failures
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i think i have fixed the Jenkins failures now

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 this looks good to me - let's merge if tests pass & then I'll do some more work on dedupe cleanup after which should put it through it's paces even more

@seamuslee001 seamuslee001 merged commit 3900218 into civicrm:master May 28, 2019
@seamuslee001 seamuslee001 deleted the fix_cache_key_column branch May 28, 2019 05:17
@eileenmcnaughton
Copy link
Contributor

Yay @seamuslee001 that will save future pain!

@colemanw
Copy link
Member

@totten - FYI I know you've done a lot of work in the cache system, so if you or @eileenmcnaughton have any extensions e.g. for the WMF caching project that query this table, they will need to be updated.

@eileenmcnaughton
Copy link
Contributor

@colemanw thanks for thinking of that but no that should not apply

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.

3 participants