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

Add support for weeks #68

Merged
merged 1 commit into from
Mar 7, 2016
Merged

Conversation

lauranjansen
Copy link
Collaborator

Fixes issue #61

Added support for weeks in the time hash
Added new tests
Adjusted existing tests

@lauranjansen lauranjansen mentioned this pull request Jan 12, 2016
@lauranjansen
Copy link
Collaborator Author

Could you perhaps look at this bundler issue to see if that fixes the build error?
rubygems/bundler#3558

@lauranjansen
Copy link
Collaborator Author

Ok, fixed the tests by updating the travis.yml file. Could someone else review the code as well before I merge it?

@redbar0n? @rvenk?

@redbar0n
Copy link
Contributor

redbar0n commented Feb 4, 2016

I looked at the code in the commit and what is there looks good to me. I don't know enough about the codebase in general to say if anything is missing.

@lauranjansen
Copy link
Collaborator Author

Thanks for taking a look!

However, because it changes the existing functionality (:highest_measure_only will now display weeks between days and months), I'm considering not merging this PR until I figure out how to bump the major version. Do you think that would be ok?

@redbar0n
Copy link
Contributor

redbar0n commented Feb 4, 2016

Yeah. You could probably submit a PR to change the readme as well, so it
corresponds. And notify @radar about it so he or someone else can include
accept it in the major version.

On Thu, Feb 4, 2016 at 5:09 PM, Lauran Jansen [email protected]
wrote:

Thanks for taking a look!

However, because it changes the existing functionality
(:highest_measure_only will now display weeks between days and months), I'm
considering not merging this PR until I figure out how to bump the major
version. Do you think that would be ok?


Reply to this email directly or view it on GitHub
#68 (comment).

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