-
-
Notifications
You must be signed in to change notification settings - Fork 710
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
deprecate persistent_workspaces in favor of persistent-workspaces #2468
Conversation
ff17400
to
2d0a00e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said in the issue, I think we should:
- Make both options parseable
- Log a warning if the user uses the old style
- After a couple of Waybar releases, we should remove the old style.
This way, no existing config would break. If we merge this in the current state, we'd have tons of people opening issues because their configs would break.
@zjeffer This new commit look better? Adds a deprecation warning and allows both configs. We can remove old config style after next release, then. |
3f10b96
to
bec707d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better! I can't test right now but I'm sure it works fine.
bec707d
to
c9e1899
Compare
Rebased to resolve merge conflicts. Still works fine, like before. |
@Alexays Should be good to merge, too. Whenever you get a chance. |
Closes: #2443
Persistent workspaces currently is the only option using this naming style, changing to match all other options.
Prompt updating config from
persistent_workspaces
topersistent-workspaces
but keep current implementation support until after next release to remove.Follow up: Update wiki entries to match new name.