-
Notifications
You must be signed in to change notification settings - Fork 2k
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
treewide: remove deprecated shell_commands module #20485
treewide: remove deprecated shell_commands module #20485
Conversation
Replaces it by shell_cmds_default where needed Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
Signed-off-by: Dylan Laduranty <dylan.laduranty@mesotic.com>
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.
Thanks, looks good!
ifneq (,$(filter shell_commands,$(USEMODULE))) | ||
# shell_commands has been renamed to shell_cmds_default, but let's keep this | ||
# for backward compatibility | ||
USEMODULE += shell_cmds_default | ||
endif |
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.
Is this really necessary?
This will break all external apps, keeping those few lines around wouldn't hurt and avoid the churn on users.
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.
This module is deprecated for more than a year already. We were supposed to remove it after 2023.07 release according to makefiles/pseudomodules.inc.mk
I guess we give enough time to our users to migrate already.
This will break all external apps, keeping those few lines around wouldn't hurt and avoid the churn on users.
The question is do we want to keep backward compatibility forever ?
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.
I'm opting for removal. If your old app really needs it, you can still use old releases of RIOT.
(I also believe it is a good thing, to annoy your users to migrate to new APIs regularly, as long as it is done in a sensible manner. A bit like Apple does it. But that's clearly an personal opinion)
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.
I'm also in favor of removal, in the end that's what our yet-to-be-documented deprecation process is for.
I'm still in favor of removing long enough deprecated stuff. |
Thanks ! |
Contribution description
This PR removes a deprecated module that should have been removed a few release ago.
I've replaced it by
shell_cmds_default
where needed.Testing procedure
CI should be enough to catch up any miss.
Issues/PRs references
None.