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

WIP - Switch add-entity command to new php format #341

Closed
wants to merge 1 commit into from

Conversation

colemanw
Copy link
Contributor

@colemanw colemanw commented Mar 14, 2024

Just some ideas/WIP so far...

@totten
Copy link
Owner

totten commented Jun 26, 2024

Good start @colemanw.

I've been playing around with this a bit, and a few things pop out:

  • I detect some ambivalence about whether to generate DAO files. I think we should stick to the earlier plan and continue making them. There are too many balls in the air. If we toss in a few more balls, then we'll never finish consolidating. (In the future, we can open #5666: Imagine a world without DAO...)
  • We have to rework tests/e2e/AddEntityTest.php -- since the actual substance+flow of the generator is changing.
  • AddEntityTest says things like: "Make sure the newly generated entity produces valid SQL schema." That makes sense for E2E test. But this can't be resolved without having (dev/core#4999) Support Entity Framework v2 for extensions #331. On the flipside, IIRC, #331 was hitting some failures because generate:entity wasn't giving the right kind of skeleton (which would be provided by this #341).
  • In other words, #331 and #341 are interdependent. They simply touch different aspects of the same project. (Project: Support extension lifecycle with schema/*.php)
  • So I'm gonna combine them (i.e. merge 341 into 331).
  • A side-effect of PhpData - Use structured document instead of regex #358 is that it should allow simpler fix for the dephell between fn() and CIVIVER=5.57.

@totten totten closed this Jun 26, 2024
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.

2 participants