Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

make zones_map private #14252

Closed
wants to merge 1 commit into from

4 participants

@mcfiredrill

I noticed there is no documentation for ActiveSupport::TimeZone.zones_map. I was using this method before to get an array of all the names of time zones:

ActiveSupport::TimeZones.zones_map(&:name)

Now zones_map seems to return a ThreadSafe::Cache instead of something I can map and call name on. So I switched to using .all:

ActiveSupport::TimeZones.all.map(&:name)

It seems like maybe zones_map wasn't something intended to be used in the public interface at all, or at least not anymore. If it is, can we write some documentation for it?

@chancancode
Owner

cc @guilleiguaran - this was changed in #11796, can you c/d if that's private API, and whether this needs to be public (but nodoc'ed)?

@arthurnn
Collaborator

:+1: this make sense to me, we actually had the same problem. we were using zones_map, and the return value changed on 4.1+ has changed. all is the public API and it should be used!

@arthurnn
Collaborator

Thanks . merged via b9ba90d on master.
and made nodoc on 4.2 a0a6e02 .

@guilleiguaran I wonder if we should also backport that to 4.1, as this should be internal anyways.

@arthurnn arthurnn closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 3, 2014
  1. @mcfiredrill

    make zones_map private

    mcfiredrill authored
This page is out of date. Refresh to see the latest.
View
15 activesupport/lib/active_support/values/time_zone.rb
@@ -377,13 +377,6 @@ def all
@zones ||= zones_map.values.sort
end
- def zones_map
- @zones_map ||= begin
- MAPPING.each_key {|place| self[place]} # load all the zones
- @lazy_zones_map
- end
- end
-
# Locate a specific time zone object. If the argument is a string, it
# is interpreted to mean the name of the timezone to locate. If it is a
# numeric value it is either the hour offset, or the second offset, of the
@@ -410,6 +403,14 @@ def [](arg)
def us_zones
@us_zones ||= all.find_all { |z| z.name =~ /US|Arizona|Indiana|Hawaii|Alaska/ }
end
+
+ private
+ def zones_map
+ @zones_map ||= begin
+ MAPPING.each_key {|place| self[place]} # load all the zones
+ @lazy_zones_map
+ end
+ end
end
private
View
3  activesupport/test/time_zone_test.rb
@@ -395,8 +395,7 @@ def test_unknown_zone_with_utc_offset
end
def test_unknown_zones_dont_store_mapping_keys
- ActiveSupport::TimeZone["bogus"]
- assert !ActiveSupport::TimeZone.zones_map.key?("bogus")
+ assert_nil ActiveSupport::TimeZone["bogus"]
end
def test_new
Something went wrong with that request. Please try again.