Update documentation for Duration#to_s #20293

davydovanton · 2015-05-25T10:52:02Z

No description provided.

kaspth · 2015-05-25T11:03:00Z

activesupport/lib/active_support/duration.rb

@@ -52,6 +52,13 @@ def ==(other)
      end
    end

+    # Returns the string with number of seconds that this Duration represents.


What about "Returns the amount of seconds a duration covers as a string."

looks better, thx! 😃

davydovanton · 2015-05-25T11:05:28Z

@kaspth fixed

kaspth · 2015-05-25T11:05:53Z

activesupport/lib/active_support/duration.rb

@@ -86,7 +93,7 @@ def eql?(other)
      Duration === other && other.value.eql?(value)
    end

-    def hash
+    def hash # :nodoc:


To me this should be left public, so people know a duration can be embedded in a collection. But don't bother documenting it. Let's rely on users knowing Ruby semantics instead.

i don't agree, this method returns hash of object like as Object#hash

2.2.2 :067 > 1.year.hash => -68723588783739193

Yes, I know. If we nodoc it will Rdoc then point to the Object#hash documentation?

hmm, i think no. What you think if we say(and remove nodoc) that the method is the same as Object#hash?

We don't have to say that. People know that already because we're overriding it. It's a standard OO convention to override a superclass method for specialization, so we don't have to tell people that.

okay, I understand, and now I think I need remove nodoc, you agree?

davydovanton · 2015-05-25T11:44:05Z

@kaspth updated

kaspth · 2015-05-25T11:44:43Z

activesupport/lib/active_support/duration.rb

@@ -52,6 +52,9 @@ def ==(other)
      end
    end

+    # Returns the amount of seconds a duration covers as a string.
+    #
+    #   1.day.to_s      # => "86400"


One space after to_s here.

Forget this, it matches all the others down the line.

oops 😓
updated

kaspth · 2015-05-25T11:48:48Z

activesupport/lib/active_support/duration.rb

@@ -52,6 +52,9 @@ def ==(other)
      end
    end

+    # Returns the amount of seconds a duration covers as a string.
+    #
+    #   1.day.to_s # => "86400"


Maybe we should link to to_i here, which has a lot more information.

davydovanton · 2015-05-25T12:03:49Z

@kaspth added #to_i to #to_s documentation

kaspth · 2015-05-25T12:33:03Z

We need to make sure Rdoc creates a link otherwise it's moot.

kaspth · 2015-05-25T12:34:21Z

Also your commit message is off now. I'm starting to feel like this might not be worth it, given all the back and forth we've been doing on this now 😅

davydovanton · 2015-05-25T13:04:25Z

@kaspth link will be created, you can see this in screenshot below. Also I updated the commit message 😃

kaspth · 2015-05-25T13:27:57Z

Alright, per our documentation guidelines we want to avoid "you"s and "your"s, so if you can find a way to rephrase that we're golden 👍

davydovanton · 2015-05-25T13:41:51Z

@kaspth complete this too 😃

Update documentation for Duration#to_s

kaspth · 2015-05-25T13:46:02Z

Alright, took a while but it's good now ❤️

davydovanton · 2015-05-25T13:49:13Z

wow, we did it 🎉
thanks for review 💚

kaspth reviewed May 25, 2015
View reviewed changes

davydovanton force-pushed the doc-duration branch from bb38fe7 to 3291b64 Compare May 25, 2015 11:05

kaspth reviewed May 25, 2015
View reviewed changes

davydovanton force-pushed the doc-duration branch from 3291b64 to fd04d52 Compare May 25, 2015 11:43

davydovanton changed the title ~~Update documentation for Duration #to_s and #hash~~ Update documentation for Duration#to_s May 25, 2015

kaspth reviewed May 25, 2015
View reviewed changes

davydovanton force-pushed the doc-duration branch from fd04d52 to 4e93299 Compare May 25, 2015 11:46

kaspth reviewed May 25, 2015
View reviewed changes

davydovanton force-pushed the doc-duration branch from 4e93299 to bf4d97b Compare May 25, 2015 12:02

davydovanton force-pushed the doc-duration branch from bf4d97b to ac60731 Compare May 25, 2015 13:03

[skip ci] Update documentation for Duration#to_s

2c72589

davydovanton force-pushed the doc-duration branch from ac60731 to 2c72589 Compare May 25, 2015 13:36

kaspth added a commit that referenced this pull request May 25, 2015

Merge pull request #20293 from davydovanton/doc-duration

7a08b2c

Update documentation for Duration#to_s

kaspth merged commit 7a08b2c into rails:master May 25, 2015

davydovanton deleted the doc-duration branch May 25, 2015 13:49

Update documentation for Duration#to_s #20293

Update documentation for Duration#to_s #20293

Uh oh!

Conversation

davydovanton commented May 25, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

kaspth commented May 25, 2015

Uh oh!

kaspth commented May 25, 2015

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

kaspth commented May 25, 2015

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

kaspth commented May 25, 2015

Uh oh!

davydovanton commented May 25, 2015

Uh oh!

Uh oh!