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

Use model attributes instead of schema in CRUD generator #470

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

WinterSilence
Copy link
Contributor

Q A
Is bugfix? yes
New feature? no
Breaks BC? no
Tests pass? yes
Fixed issues

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@samdark samdark added this to the 2.2.4 milestone Sep 27, 2021
@samdark samdark added the type:enhancement Enhancement label Sep 27, 2021
@samdark samdark changed the title Update index.php Use model attributes instead of schema in CRUD generator Sep 27, 2021
@samdark samdark requested a review from a team September 27, 2021 21:15
Copy link
Member

@machour machour left a comment

Choose a reason for hiding this comment

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

Great idea !

Would you please post an example output in comment ?

src/generators/crud/default/views/index.php Show resolved Hide resolved
src/generators/crud/default/views/index.php Show resolved Hide resolved
@machour
Copy link
Member

machour commented Sep 28, 2021

@samdark shouldn't we have tests in this extension, including & comparing outputs ? That would make it easier to track changes

@samdark
Copy link
Member

samdark commented Sep 28, 2021

@machour yeah but we don't have any :)

@rob006
Copy link
Contributor

rob006 commented Sep 28, 2021

yeah but we don't have any :)

Maybe we should start with adding these, and only then start refactoring this extension. Right now reviewing such PRs is a nightmare - it is often really hard to understand what are the actual implications of some changes.

@samdark samdark removed this from the 2.2.4 milestone Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants