Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement CustomRelTime #33

Closed
wants to merge 1 commit into from
Closed

Implement CustomRelTime #33

wants to merge 1 commit into from

Conversation

dustin
Copy link
Owner

@dustin dustin commented Apr 21, 2016

for #25


This change is Reviewable

@dmitshur
Copy link
Collaborator

Is this based on #26? How similar is it, and what are the differences?

@dustin
Copy link
Owner Author

dustin commented Apr 21, 2016

Heh, well, no. I'd forgotten about that one. :/ I just sat down on the
bus and wrote it from scratch today. That's a little embarrassing. I'm
glad I sent it out for review. :)

On Wed, Apr 20, 2016 at 11:05 PM Dmitri Shuralyov notifications@github.com
wrote:

Is this based on #26 #26? How
similar is it, and what are the differences?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#33 (comment)

@dmitshur
Copy link
Collaborator

I think having 2 implementations will help us find a more optimal final solution, by taking best ideas/pieces from both. But it does mean a little more work consolidating them.

@dustin
Copy link
Owner Author

dustin commented Apr 21, 2016

Apologies for doing that. I just saw a note and was bored on the bus. Feel free to abandon mine, or whatever makes sense.

@dmitshur
Copy link
Collaborator

I don't see a need to apologize. I think your implementation is likely better to go with, since you wrote this package in the first place and have better context on what fits in. But I haven't reviewed it in detail yet.

@dustin
Copy link
Owner Author

dustin commented Apr 21, 2016

Please don't take that into consideration. If anything, someone else's
work is more valuable than mine. I already did a bunch.

Whatever makes sense. There are at least three options here. :)

On Thu, Apr 21, 2016, 00:02 Dmitri Shuralyov notifications@github.com
wrote:

I don't see a need to apologize. I think your implementation is likely
better to go with, since you wrote this package in the first place and have
better context on what fits in. But I haven't reviewed it in detail yet.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#33 (comment)

@dustin dustin closed this Jul 21, 2016
@dmitshur
Copy link
Collaborator

@dustin, according to #26 (comment), you ended up merging #26 with a modified version of this commit on top.

So it should be safe to delete the custime branch now. I'll do that in order to clean up. If it's needed again, it can be restored from this PR.

@dmitshur dmitshur deleted the custime branch February 28, 2017 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants