Day 3 - Let's get rid of a warning

Today’s exercise: get rid of a warning you’ve gotten used to ignoring.

Chances are, your code prints at least one warning during one of the following:

  • Booting your code
  • Running your tests
  • Installing dependencies
  • Deploying your app

These warnings are easy to learn to ignore with time, but there are two problems with that:

  1. Many warnings (like a deprecation warning) will turn into full-blown issues later on. This will tend to happen at inconvenient times.

  2. They act as a “broken window”: an indication that your codebase isn’t receiving the fastidious love and care it needs. These small messes quietly demonstrate to your team that messes are tolerated, and can often result in more. (More on “broken windows” https://blog.codinghorror.com/the-broken-window-theory/)

So, today’s task is to try to nuke at least one warning.

If you boot your app, run your tests, install your dependencies, and deploy with no warnings, congrats! Enjoy your day off.

Otherwise, see if you can eliminate a broken window.

Good luck!

2 Likes

This one is great! I’m always surprised by people’s tolerance for spammy warning messages, especially when running tests, or starting up dev environments/servers. That said, my dislike of these messages means that I couldn’t find any to get rid of.

1 Like

So good! Had to update one gem to use master (as there hasn’t been new releases after fixing the warnings) and add a temporary workaround while another gets new release too.

As a tip I could say that remember to write reasoning into commit’s message, it’s helpful in future. I usually write something like Fixes deprecation warning: [copy pasted warning]. Can be reverted once new version has been released.

The Sensio\Bundle\DistributionBundle\Controller\ConfiguratorController class extends Symfony\Component\DependencyInjection\ContainerAware that is deprecated since version 2.8, to be removed in 3.0. Use the ContainerAwareTrait instead.

Finally took the time to remove the last deprecation warning of my Symfony 2.8 LTS installation and now will be able to upgrade to the next LTS version 3.4 later this year. :+1:

https://github.com/codevise/pageflow/pull/943

Replaced FactoryGirl gem which has been renamed to FactoryBot. Then realized that this will be a breaking change for our engine since plugins will need to update as well. Ran out of time and will leave this to be merged for the next major version.

Hi,

The only warning I have at the moment is from Puma:

WARNING: Detected 1 Thread(s) started in app boot:

I did some digging and it can safely be ignored.

I used the extra time to improve the README :wink:

Good reading though :+1:

After great success (that I didn’t report here) removing all TODOs, this one has beaten me (within the time limit). The only warnings I’ve got are two timezone related ones during a test run and I haven’t managed to get to the bottom of them in 20 minutes. :frowning:

I’m going to have to wait until Friday to tackle the next two challenges but I’m looking forward to them.

I got lucky on this one, my current project doesn’t produce any warnings.

As a general rule, I’d advocate that all console.log messages in JavaScript code falls in scope of this exercise, except possibly during active debugging. Other than that, if a message needs to be generated it should be logged somewhere where the dev team will actually see it, and the end user’s JS console is not that place :slight_smile:

2 Likes

I finally managed to get rid of an annoying warning that Ruby 2.4 introduced.

It warns when delegating to private methods.

Example:

app/controllers/question_tags_controller.rb:94:in `question_breadcrumb': QuestionTagsManager#question_summary at /Users/aaronmcadam/workspace/projects/dash-api3/app/services/question_tags_manager.rb:73 forwarding to private method Question#summary

Ordinarily this warning makes sense, however the affected objects use method_missing it’s technically private.

The solution is easy enough: follow best practice and define #respond_to_missing? too.

Thanks to a PR on shoulda-matchers for the solution!

Upgrading my Rails app to use Ruby 2.4.2 recently caused a deprecation warning to appear from a gem. The gem wasn’t updated for at least a year so it’s no surprise the deprecation warning appears. For me I always resolve warnings whenever I was free as deprecation warning tends to stack up and annoys me.

Done! I’m the same on the upgrade to Ruby 2.4.2. I was getting ‘Fixnum is deprecated’ on my test suite. A few edits and we are running perfectly green. :green_heart:

1 Like

This is an interesting thought. I’ll have to look into how to pipe JS console logs into our main Rails logs.

1 Like

Done :+1: Got rid of a lingering deprecation warning in our test suite.

Fixed a startup bootstraping error that has been bugging me recently. It didn’t impact overall functionality, but a stacktrace being spit out on startup was always scary.

Our Jenkins builds started failing some time ago with an error coming from the GitHub plugin that reports build status to pull requests. The error happened after it had reported the status to GitHub, so it didn’t bother our workflow too much, but it was annoying when you went to look at the Jenkins UI as all builds seemed to be failed. Managed to fix this by updating the plugin and now builds are green in Jenkins again :sunny:

Fixed a deprecation about referencing a top-level constant incorrectly in Ruby. We are locked on a branch of a Gem for reasons. Because the prefix was that specific gem, I had fallen into accepting the deprecation as fate, because upgrading the branch is a tedious process. I tried removing the prefix, tests pass, and the warning is gone. Huzzah! :mute:

This one was definitely tougher to get done in the 20 minutes. I removed a single warning on startup of one of our microservices where a dependency was out of date for the version of Django (Python web framework) we’re using. We had been using this dependency, updated Django which caused that dependency to break, but there was a workaround for that dependency which fixed the problem but caused a warning on startup of the service. Now that the dependency has been updated/fixed, we no longer need the workaround, so I removed that and warning now gone.

Figuring all that out was the bulk of the 20 minutes. The actual code change was maybe a minute or two of work. :confused:

1 Like

When I saw this challenge, I knew exactly which of our apps I would be working with. I ended up being able to remove an out of date development dependency we don’t use.

Like @hasanen mentioned, I made sure to put something in my commit explaining why I removed the dependency and noted it could be added back if there are any issues down the road.

I found another warning message that I hadn’t found before that is less offensive looking. Unfortunately, I could not get to the bottom of the problem in 20 minutes.

1 Like

Done

This was a simple fix for me but took a bit of time. A core package I rely on recently deprecated the main function it exports in favour of a new name and small syntax change.

I went through my entire application and got rid of the deprecated function and replaced it with the new one.

No more console messages! :ok_hand:

Fixed one warning and uncovered a bug in our tests by checking out another warning!

1 Like