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.

Sign up for GitHub

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

Jump to bottom

Move scaffolding commands into System module #270

Closed
bennothommo opened this issue Aug 18, 2021 · 6 comments · Fixed by #471
Closed

Move scaffolding commands into System module #270

bennothommo opened this issue Aug 18, 2021 · 6 comments · Fixed by #471
Milestone
v1.2.0

Comments

@bennothommo
Copy link
Member

bennothommo commented Aug 18, 2021 •
edited
Loading

Package targeted

Both

Description

Currently, the scaffolding commands in Winter CMS (create:plugin, create:controller, etc.) reside in the Storm library for historical purposes.

It may, however, make more sense for these to reside in the System module in Winter CMS, as these functions relate directly to Winter CMS functionality, and require a Winter CMS install to be used effectively. They would not be able to be used in isolation with the Storm library.

If accepted, I intend to put forward a PR that moves all the create:* commands into the System module, but leaving the base Scaffolding classes and Service Provider within the Storm library.

The main benefit of this change will be that we can write test cases for the scaffold commands, as currently these are not tested within the Storm library, ostensibly because it's impossible to load the plugin architecture within the Storm library tests.

Will this change be backwards-compatible?

Mostly backwards-compatible, as most people will just use the commands directly, but it would break any custom implementations of these commands which may extend the original command.
@bennothommo bennothommo added Type: Conceptual Enhancement needs review Issues/PRs that require a review from a maintainer labels Aug 18, 2021
@bennothommo bennothommo changed the title Move scaffolding commands in System module Move scaffolding commands into System module Aug 18, 2021
@LukeTowers
Copy link
Member

The theme commands should probably be moved to the CMS module then. Should we move the specific scaffolding commands into each applicable module? i.e.

System Module:

  • plugin
  • command
  • model
  • settings (when implemented)

Backend Module:

  • Controller
  • FormWidget
  • ReportWidget

CMS Module:

  • theme
  • component

@bennothommo
Copy link
Member Author

Yeah, I can definitely do it that way. I assumed we would just put them in System as those commands have traditionally been available at all times given that they've been provided by the Storm library, but it does make sense to make them only available if needed.

@github-actions
Copy link

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue.
If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Oct 19, 2021
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Oct 19, 2021
@github-actions
Copy link

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue.
If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Dec 19, 2021
@bennothommo bennothommo removed the stale Issues/PRs that have had no activity and may be archived label Dec 20, 2021
@github-actions
Copy link

This issue will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this issue is still relevant or you would like to see it actioned, please respond and we will re-open this issue.
If this issue is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@github-actions github-actions bot added the stale Issues/PRs that have had no activity and may be archived label Feb 19, 2022
@LukeTowers LukeTowers removed the stale Issues/PRs that have had no activity and may be archived label Feb 19, 2022
@LukeTowers LukeTowers added this to the v1.2.0 milestone Feb 19, 2022
@LukeTowers LukeTowers mentioned this issue Feb 19, 2022
Merged
71 tasks
@bennothommo bennothommo mentioned this issue Feb 23, 2022
Merged
@bennothommo bennothommo linked a pull request Feb 23, 2022 that will close this issue
Move scaffold commands #471
Merged
LukeTowers pushed a commit that referenced this issue Feb 23, 2022
@bennothommo
Move scaffold commands (#471)
e3fecdb 
Fixes #270. See wintercms/storm@c4cd440

Moves the Scaffolding commands out of Storm, and into the modules that the scaffold generates code for.

As discussed in the issue above, this makes more sense to keep Storm agnostic, and allows for the commands to be hidden if a particular module is not used in a Winter instance.
@LukeTowers LukeTowers added Status: Completed maintenance PRs that fix bugs, are translation changes or make only minor changes and removed Type: Conceptual Enhancement needs review Issues/PRs that require a review from a maintainer labels Feb 23, 2022
@LukeTowers
Copy link
Member

Closed by e3fecdb

@bennothommo bennothommo removed the maintenance PRs that fix bugs, are translation changes or make only minor changes label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging a pull request may close this issue.

Move scaffold commands wintercms/winter
2 participants
@LukeTowers @bennothommo