Skip to content

Rename justification settings to just settings#6652

Open
teunbrand wants to merge 6 commits into
tidyverse:mainfrom
teunbrand:justification_to_just
Open

Rename justification settings to just settings#6652
teunbrand wants to merge 6 commits into
tidyverse:mainfrom
teunbrand:justification_to_just

Conversation

@teunbrand

Copy link
Copy Markdown
Collaborator

This PR aims to fix #6607.

I'll reiterate that this is just for consistency purposes.
I know from previous revdepchecks that some packages save their themes to objects instead of regenerating them by calling theme().
Other packages have expect_silent() in their test, which is incompatible with lifecycle changes.
Such packages are at risk of being negatively affected.

@thomasp85 thomasp85 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this break every piece of code that uses the old name

@teunbrand

Copy link
Copy Markdown
Collaborator Author

If the code is building a <theme> object, no. If they're reading out a <theme> object, it might. I could be persuaded that this is a risky move that is not worth the intended benefit of consistency.

@thomasp85

Copy link
Copy Markdown
Member

Ah - I see now how you deal with the old spelling inside the code... I'm fine with putting this in the code base - if you want we can run a revdepcheck on it before merging?

@teunbrand

Copy link
Copy Markdown
Collaborator Author

Yeah I think a revdepcheck would be wise for this.

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.

Could the legend.box.just be named legend.box.justification for consistency?

2 participants