scoutapp / scout_apm_elixir

ScoutAPM Elixir Agent. Supports Phoenix and other frameworks.
https://scoutapm.com
Other
36 stars 20 forks source link

rename *seconds to *second, move System.cwd to File.cwd to fix all deprecations on 1.8.0 #72

Closed fatboypunk closed 5 years ago

fatboypunk commented 5 years ago

Ran the tests with 1.8.0-rc.1 and got some deprecation warnings, this should fix them.

cschneid commented 5 years ago

Hi, thank you for the PR, I'll need to take a bit to read it over and see if this is mergeable with our restrictions.

One big thing is that we need to continue to support older Elixir versions (1.4 onward is what we advertise), do you know if these deprecation changes are now using new APIs that didn't exist in older versions?

In addition, my quick skim looks like the seconds -> second change will be a breaking change to our external API. What's the motivation for changing the Duration type?

Then most of the other changes appear to be related to (finally) having mix format run against the codebase.

fatboypunk commented 5 years ago

You're right, I changed too much. This PR now only changes the parts that gave the warning.

One big thing is that we need to continue to support older Elixir versions (1.4 onward is what we advertise), do you know if these deprecation changes are now using new APIs that didn't exist in older versions?

I've checked and what this PR changes to already existed in 1.4, also the tests seem to work for 1.4

mitchellhenke commented 5 years ago

@fatboypunk thank you!

wcpines commented 5 years ago

This seems worthy of a release. It would be nice to not have logs outputting the following warning during tests (or any other local dev).

warning: deprecated time unit: :microseconds. A time unit should be :second, :millisecond, :microsecond, :nanosecond, or a positive integer

cschneid commented 5 years ago

@wcpines I just pushed a new point release of the agent - v0.4.10 with these changes. Give it a try?

wcpines commented 5 years ago

Works, thank you!