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

Time intervals #44

Merged
merged 4 commits into from
Aug 31, 2014
Merged

Time intervals #44

merged 4 commits into from
Aug 31, 2014

Conversation

tanriol
Copy link
Member

@tanriol tanriol commented Oct 8, 2013

Make approximate time displays more precise

@ancestorak
Copy link
Collaborator

Thanks!

  1. I intended the RelativeDate object to be similar to the standard Date object in terms of its API. Therefore, I would prefer to keep the delta* getters and have intervals use getters as well (e.g. RelativeDate.intervalDays etc.)

  2. The terms delta and interval aren't very clear because it's hard to figure out the difference without reading the implementation, but I can't think of anything better myself. Let's at least add a comment on top of the class that explains the difference.

  3. By the way, please don't overwrite the original function arguments like you did with aDivisor. Instead, define a new variable, called divisor for example. Brief uses a convention that variables prefixed with the letter "a" always represent the original function arguments. This doesn't matter here because we're gonna use getters anyway, it's just a note for the future.

  4. You can use this handy tool to convert existing issues into pull requests.

@tanriol
Copy link
Member Author

tanriol commented Oct 8, 2013

  1. I intended the RelativeDate object to be similar to the standard Date object in terms of its API. Therefore, I would prefer to keep the delta* getters and have intervals use getters as well (e.g. RelativeDate.intervalDays etc.)

The previous time you preferred not to have an extra set of accessor functions. OK, if now you wish so...

  1. The terms delta and interval aren't very clear because it's hard to figure out the difference without reading the implementation, but I can't think of anything better myself. Let's at least add a comment on top of the class that explains the difference.

What about deltaHours and deltaHourSteps?

@ancestorak
Copy link
Collaborator

The previous time you preferred not to have an extra set of accessor functions. OK, if now you wish so...

I thought it would look better but now I see I was wrong. Sorry to keep changing my mind.

What about deltaHours and deltaHourSteps?

I assume the former corresponds to the "interval" and the latter to "delta"? Sounds good!

@hurda
Copy link

hurda commented Oct 29, 2013

Any ETA on this? I've set the update-interval to 3 hours, and it's always a bit weird to see "Feeds were last updated 3 hours ago" in the tooltip.

@ancestorak
Copy link
Collaborator

@tanriol I can finish it up this week if you don't have the time.

@tanriol
Copy link
Member Author

tanriol commented Feb 23, 2014

This should fix #31. By the way, what about releasing 2.0b1? The alpha is pretty broken.

@tanriol
Copy link
Member Author

tanriol commented Jul 25, 2014

@ancestorak, is this ok to be merged?

@ancestorak
Copy link
Collaborator

Yes, let's merge it.

By the way, I'm back and I'll be devoting time to Brief again.

tanriol added a commit that referenced this pull request Aug 31, 2014
Use more correct display for time intervals
@tanriol tanriol merged commit b433a15 into brief-rss:master Aug 31, 2014
@tanriol tanriol deleted the time-intervals branch August 31, 2014 21:07
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.

3 participants