Skip to content

added fct_lump_inorder() function#193

Open
dan-reznik wants to merge 2 commits into
tidyverse:mainfrom
dan-reznik:master
Open

added fct_lump_inorder() function#193
dan-reznik wants to merge 2 commits into
tidyverse:mainfrom
dan-reznik:master

Conversation

@dan-reznik

@dan-reznik dan-reznik commented Apr 1, 2019

Copy link
Copy Markdown

dear maintainer, i added my own solution to issue #187. this is my first contrib to the tidyverse and my first pull request ever. one thing missing: i don't know how to add this function to the vignette.

@batpigandme

Copy link
Copy Markdown
Member

Hi @dan-reznik. Are you talking about the vignette here? https://github.com/tidyverse/forcats/blob/master/vignettes/forcats.Rmd

If not, the README will be built, and changes propagated as part of the pkgdown site automatically after merging.

Just for future reference, it'll be easier workflow-wise in the future if you use a branch for pull requests, rather than "master" in your fork. I'm working on a write-up for this workflow right now, but just thought I'd mention it. 👍

@dan-reznik

dan-reznik commented Apr 1, 2019

Copy link
Copy Markdown
Author

Dear Mara -- I noticed fct_lump(), the "closest sibling" to the function I added (fct_lump_inorder()) is not mentioned in forcat's .Rmd vignette. So I guess I will leave it as is. Also, yes, looking fwd to your writeup on how to submit pull requests on an alternate branch.

@hadley

hadley commented Feb 28, 2020

Copy link
Copy Markdown
Member

Fixes #187

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