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

minimal support for raw vectors. #455

Merged
merged 3 commits into from Feb 20, 2018

Conversation

@romainfrancois
Copy link
Member

commented Feb 5, 2018

cleaner version of #345
I manually applied the changes from the broken PR to the current master

@romainfrancois

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2018

🤔 the failures seem related to the .default argument in map:

https://travis-ci.org/tidyverse/purrr/jobs/337610412#L1150

@hadley

This comment has been minimized.

Copy link
Member

commented Feb 8, 2018

Yeah, that was my fault - can you please merge/rebase from master?

@romainfrancois

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2018

So I have

$ git remote -v
origin	https://github.com/romainfrancois/purrr.git (fetch)
origin	https://github.com/romainfrancois/purrr.git (push)
upstream	https://github.com/tidyverse/purrr.git (fetch)
upstream	https://github.com/tidyverse/purrr.git (push)

and then I've done

git fetch upstream
git merge upstream/master
git push

I might have to properly learn about rebase some day.

@romainfrancois

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2018

I guess I was close with my rebase attempts and all I needed really was the -f as described in https://gist.github.com/ravibhure/a7e0918ff4937c9ea1c456698dcd58aa

DESCRIPTION Outdated
@@ -25,4 +25,5 @@ Suggests:
VignetteBuilder: knitr
LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 6.0.1

This comment has been minimized.

Copy link
@hadley

hadley Feb 12, 2018

Member

Can you please use the released version of roxygen2?

@hadley

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

Can you please also add a bullet to NEWS?

romainfrancois added some commits Feb 5, 2018

@romainfrancois

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2018

I think I managed to do a clean rebase this time 🎉

@hadley

This comment has been minimized.

Copy link
Member

commented Feb 19, 2018

Looks good!

Can you please add a bullet to NEWS? It should briefly describe the change (starting with name of the function), and crediting yourself with (@yourname, #issuenumber).

@romainfrancois

This comment has been minimized.

Copy link
Member Author

commented Feb 20, 2018

Done. Does it make sense to do the same for complex for completeness or is it a maintenance burden down the line ?

@hadley

This comment has been minimized.

Copy link
Member

commented Feb 20, 2018

I don't see the benefit to doing it until someone has expressed a need.

@hadley hadley merged commit 84ce1ad into tidyverse:master Feb 20, 2018

3 checks passed

codecov/patch 100% of diff hit (target 95.74%)
Details
codecov/project 95.83% (+0.09%) compared to b3c29c8
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley

This comment has been minimized.

Copy link
Member

commented Feb 20, 2018

Thanks!

@romainfrancois romainfrancois referenced this pull request Feb 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.