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

wrap_text overlaps with other work #716

Closed
Melkiades opened this issue Sep 29, 2022 · 3 comments · Fixed by #735
Closed

wrap_text overlaps with other work #716

Melkiades opened this issue Sep 29, 2022 · 3 comments · Fixed by #735
Assignees
Labels
discussion question Further information is requested sme

Comments

@Melkiades
Copy link
Contributor

Melkiades commented Sep 29, 2022

By chance, I noticed there is a function in wrap_text.R that is no longer used anywhere. I was wondering if it still makes sense to have it and/or if we should adapt/move it somewhere else. I think conceptually it is more appropriate to be a function in formatters, where I am working on a similar implementation (unaware of the above file) here: PR#46.

The author is @waddella. Any suggestion on how to proceed @gmbecker @shajoezhu @waddella?

I am checking downstream dependencies now. For me, it is fine also to keep it as it is. I was just wondering about the possible duplication with formatters'function.

I could not find any downstream dep that uses this function (also, it was rendered internal and nothing broke).

@Melkiades Melkiades added question Further information is requested sme discussion labels Sep 29, 2022
@Melkiades Melkiades changed the title wrap_text is no longer used and overlaps with other work wrap_text overlaps with other work Sep 29, 2022
@Melkiades
Copy link
Contributor Author

I suggest adding the deprecated badge and removing it in the next increment.

@Melkiades Melkiades self-assigned this Oct 17, 2022
@shajoezhu
Copy link
Contributor

let's do that. I wil follow up with Adrian and Gabe tonight on this as well

@insightsengineering insightsengineering deleted a comment from shajoezhu Oct 31, 2022
@Melkiades
Copy link
Contributor Author

any update regarding this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion question Further information is requested sme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants