Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(367)

Issue 10538105: Make isUtc a getter, change some method names in Date. (Closed)

Created:
8 years, 6 months ago by floitsch
Modified:
8 years, 6 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org, sorinmocanu, kasperl
Visibility:
Public.

Description

Make isUtc a getter, change some method names in Date. Weekday is now 1 based. isUtc is now a getter. Date.hours -> Date.hour Date.minutes -> Date.minute Date.seconds -> Date.second Date.milliseconds -> Date.millisecond Date.fromEpoch -> Date.fromMillisecondsSinceEpoch Date.value -> Date.millisecondsSinceEpoch Date.MON == 1 (up to Date.SUN == 7). Fixes issue 3444, 1985. Committed: https://code.google.com/p/dart/source/detail?r=8899

Patch Set 1 #

Patch Set 2 : Cosmetic changes. #

Patch Set 3 : rebase #

Total comments: 14

Patch Set 4 : Address comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+766 lines, -686 lines) Patch
M corelib/src/date.dart View 1 2 3 5 chunks +43 lines, -35 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core_test/src/com/google/dart/tools/core/internal/model/testsource/Clock.dart View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core_test/src/com/google/dart/tools/core/internal/model/testsource/CoreRuntimeTypesTest.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M lib/compiler/implementation/lib/js_helper.dart View 1 2 1 chunk +10 lines, -9 lines 0 comments Download
M lib/compiler/implementation/lib/mockimpl.dart View 5 chunks +72 lines, -58 lines 0 comments Download
M lib/compiler/implementation/ssa/tracer.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M lib/math/random.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/file_impl.dart View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M runtime/bin/http_utils.dart View 1 chunk +7 lines, -7 lines 0 comments Download
M runtime/bin/timer_impl.dart View 2 chunks +3 lines, -2 lines 0 comments Download
M runtime/lib/date.dart View 1 2 10 chunks +119 lines, -92 lines 0 comments Download
M samples/chat/chat_server_lib.dart View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M samples/chat/dart_client/chat.dart View 1 chunk +6 lines, -6 lines 0 comments Download
M samples/swarm/DataSource.dart View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M samples/third_party/dromaeo/common/BenchUtil.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/third_party/dromaeo/tests/RunnerSuite.dart View 1 chunk +3 lines, -3 lines 0 comments Download
M samples/total/client/DateTimeUtils.dart View 1 2 1 chunk +1 line, -1 line 0 comments Download
M samples/total/client/Formats.dart View 2 chunks +9 lines, -9 lines 0 comments Download
M samples/total/client/SpreadsheetPresenter.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/ui_lib/base/AnimationScheduler.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/ui_lib/touch/TimeUtil.dart View 1 chunk +1 line, -1 line 0 comments Download
M samples/ui_lib/util/DateUtils.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/co19/co19-leg.status View 1 2 1 chunk +39 lines, -29 lines 0 comments Download
M tests/co19/co19-runtime.status View 1 2 1 chunk +39 lines, -29 lines 0 comments Download
M tests/corelib/core_runtime_types_test.dart View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/corelib/date_time4_test.dart View 1 chunk +20 lines, -20 lines 0 comments Download
M tests/corelib/date_time5_test.dart View 1 chunk +37 lines, -37 lines 0 comments Download
M tests/corelib/date_time6_test.dart View 2 chunks +4 lines, -4 lines 0 comments Download
M tests/corelib/date_time_test.dart View 4 chunks +320 lines, -313 lines 0 comments Download
M tests/language/issue4157508_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M tests/standalone/io/multiple_timer_test.dart View 1 2 2 chunks +8 lines, -8 lines 0 comments Download
M tests/standalone/io/timer_repeat_test.dart View 1 2 2 chunks +3 lines, -3 lines 0 comments Download
M tests/standalone/io/timer_test.dart View 1 2 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
floitsch
Once I get an LGTM I will announce the breaking change on the mailing list.
8 years, 6 months ago (2012-06-15 14:27:11 UTC) #1
Lasse Reichstein Nielsen
LGTM (I trust the changes that are merely renames are correct) https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.dart File corelib/src/date.dart (left): ...
8 years, 6 months ago (2012-06-19 14:32:18 UTC) #2
floitsch
8 years, 6 months ago (2012-06-20 08:25:24 UTC) #3
https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.dart
File corelib/src/date.dart (left):

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:46: [int month,
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> Is "month" zero or one-based?
1-based (added comment).

> Are overflowing values allowed? (e.g, month 13 being January of the next year)
not yet, but I intend to allow it:
http://dartbug/2771

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:47: int day,
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> Should month and day really be optional? It makes little sense to me to create
a
> Date without specifying an actual date. 
See http://dartbug/2582.
Apparently requiring month and day is less useful.

> At least give them default values of 1 and 1 when possible.
They all default to the smallest valid value. (which would be 1 and 1).
> (Personally, I think you should drop the split between Date and
> DateImplementation, and just include the implementation here - there won't be
> any other (non-test) implementation of Date anyway).
Another CL.

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.dart
File corelib/src/date.dart (right):

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:24: static final int SUN = 7;
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> Are there definitions for JAN...DEC? Preferably one-based?

one line below?

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:156: * Returns the hour in the day [0..23].
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> "in" -> "into". There are always 24 hours in the day :)

Done.

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:161: * Returns the minute in the hour [0...59].
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> "in" -> "into" again.

Done.

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:166: * Returns the second in the minute [0...59].
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> And again.
> We are absolutely certain we won't need leap seconds?
I think so.
If leap-seconds are an issue some systems (for example Android) apparently use a
special timezone that takes leap-seconds into account.

https://chromiumcodereview.appspot.com/10538105/diff/4001/corelib/src/date.da...
corelib/src/date.dart:171: * Returns the millisecond in the second [0...999].
On 2012/06/19 14:32:18, Lasse Reichstein Nielsen wrote:
> And again.

Done.

Powered by Google App Engine
This is Rietveld 408576698