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/2493 #2494

Merged
merged 6 commits into from
Dec 12, 2017
Merged

Issue/2493 #2494

merged 6 commits into from
Dec 12, 2017

Conversation

mehul0810
Copy link
Contributor

Description

This PR resolved #2493

How Has This Been Tested?

I've tested this by setting email_access_installed to false and then confirming whether the notice still appears in error log or not.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.


// Columns added properly
if ( $query ) {
give_update_option( 'email_access_installed', 1 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mehul0810 Write automatic upgrade to remove email_access_installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ravinderk Added Automatic Upgrade as well as improved the functioning of creating columns as discussed over call.

@ravinderk ravinderk merged commit aeece2b into impress-org:release/1.8.18 Dec 12, 2017
*
* @return bool
*/
public function is_column_exists( $column_name ) {
Copy link
Member

Choose a reason for hiding this comment

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

@mehul0810 I'd love to get this renamed to does_column_exist( $column_name ) pre-1.8.18 release

Copy link
Member

Choose a reason for hiding this comment

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

@mehul0810 I'll update it

DevinWalker added a commit that referenced this pull request Dec 20, 2017
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.

3 participants