Skip to content

Make tzinfo opt-in. #25

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

Merged
merged 1 commit into from
Oct 9, 2015
Merged

Make tzinfo opt-in. #25

merged 1 commit into from
Oct 9, 2015

Conversation

klausenbusk
Copy link
Contributor

Not sure if it should be opt-in or opt-out, also a little unsure about the env name MYSQL_LOAD_TIME_ZONE_TABLES. I think it should be opt-in.
#24 depends on this.

@klausenbusk klausenbusk mentioned this pull request Sep 17, 2015
@md5
Copy link

md5 commented Sep 24, 2015

Seems like this needs to be an opt-out to avoid breaking existing users who are now relying on the DB having the time zones loaded. I know I personally ran into this with the mysql image one some point. I didn't find any user reports that motivated docker-library/mysql#80, but I seem to recall that there were some.

@klausenbusk
Copy link
Contributor Author

How do this look?

@md5
Copy link

md5 commented Sep 25, 2015

LGTM. Let's see what the maintainers have to say.

@klausenbusk
Copy link
Contributor Author

Any update?

@klausenbusk
Copy link
Contributor Author

Update?

@yosifkit
Copy link
Contributor

yosifkit commented Oct 9, 2015

LGTM

@tianon
Copy link
Contributor

tianon commented Oct 9, 2015

I like the feature, but I'm not so sure about the name. 😕

I'm thinking maybe something slightly more descriptive of exactly what's being skipped, like MYSQL_INITDB_SKIP_TZINFO. WDYT?

@klausenbusk
Copy link
Contributor Author

but I'm not so sure about the name.

Me either.

I'm thinking maybe something slightly more descriptive of exactly what's being skipped, like MYSQL_INITDB_SKIP_TZINFO. WDYT?

Look fine to me, I can update the PR now, if it okay?

@tianon
Copy link
Contributor

tianon commented Oct 9, 2015 via email

@klausenbusk
Copy link
Contributor Author

Done, also not sure about the commit message. Maybe it fine.

@tianon
Copy link
Contributor

tianon commented Oct 9, 2015

¯\_(ツ)_/¯

LGTM

@yosifkit
Copy link
Contributor

yosifkit commented Oct 9, 2015

Commit message seems fine.

yosifkit added a commit that referenced this pull request Oct 9, 2015
@yosifkit yosifkit merged commit c2494f9 into MariaDB:master Oct 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants