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

[Fix #373] Add 'date' accessors #380

Closed
wants to merge 1 commit into from
Closed

[Fix #373] Add 'date' accessors #380

wants to merge 1 commit into from

Conversation

joethorley
Copy link
Contributor

We will also need some tests for this functions. Could you please add a few? Also please look at #355. I think using make_datetime and then as.Date might get us into trouble.

I have added some tests. Also I avoid the problem of using make_datetime and then as.Date raised in by pasting Year-Month-Day to a character and then using as.Date.

Note the date function provides the make_date function proposed in issue #355 (which @hadley suggested be called as_date)

For more discussion about the date and date<- functions see #373.

@vspinu
Copy link
Member

vspinu commented Jan 30, 2016

Two notes on working with git:

  • You don't need to open a new issue each time you re-write your commits. Just do git push --force. Github will update the pull request automatically.
  • Each commits must deal with one issue. That makes it easy to navigate in the history. Could you please squash your two commits into one. If you are using R-studio or some other git-aware editor, there must be a way to do an interactive rebase and squash commits interactively. Otherwise from command line follow these instructions.

@joethorley
Copy link
Contributor Author

Well I squashed but in the process I managed to introduce conflicts!
Sorry.
I'm going to cancel this pull request and retry fresh based on what I have learned!

@joethorley joethorley closed this Feb 3, 2016
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