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 a number of warnings and change log levels #987

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Conversation

joshsmith
Copy link
Contributor

What's in this PR?

  • Fixes warnings about migrations relying on :datetime. I saw no harm in changing them to :utc_datetime instead.
  • Changes Logger.warn to Logger.info in a few places
  • Removes a few compiler warnings and does some quick reformatting where needed

@joshsmith
Copy link
Contributor Author

It works! Only warning (compiler or otherwise) comes from upstream dep:

Terminal output

Copy link
Contributor

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

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

Nice @joshsmith!

Just for reference, this is a really good article if you haven't seen it.

https://www.amberbit.com/blog/2017/8/3/time-zones-in-postgresql-elixir-and-phoenix/

@@ -3,7 +3,7 @@ defmodule CodeCorps.Repo.Migrations.AddStripeCustomersCardsTables do

def change do
create table(:stripe_customers) do
add :created, :datetime
add :created, :utc_datetime
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshsmith no need to replace field :created, :utc_datetime as well in the model?

Copy link
Contributor

@snewcomer snewcomer Sep 27, 2017

Choose a reason for hiding this comment

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

One thing I have seen done is for other models

@timestamps_opts [type: :utc_datetime] in the model file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you mind opening an issue for this? I'd like to migrate everything over to it specifically. We're lucky in the sense that the servers are all on UTC time by default so it shouldn't be a pain to migrate. But this is something that needs done ASAP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I'm just trying to fix compiler warnings that will always be there unless we fix old migrations. That one was from Oct 2016!

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. I'll open an issue.

@joshsmith joshsmith merged commit 327cddd into develop Sep 27, 2017
@joshsmith joshsmith deleted the fix-warnings branch September 27, 2017 18:42
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.

None yet

2 participants