search

lonnieezell / Bonfire2

CodeIgniter 4-based application skeleton
MIT License
127 stars 49 forks source link

Update HasMeta entity to correct errors #440

Closed dgvirtual closed 4 months ago

dgvirtual commented 4 months ago

preventing it working with custom classes

Here I implement a different approach that was suggested in this comment in previous pull request #428.

@lonnieezell, would that work? I see deptrac and phpstan errors, but they are not related to this PR afaik.

datamweb commented 4 months ago

would that work? I see deptrac and phpstan errors, but they are not related to this PR afaik.

I sent PR #441 .

dgvirtual commented 4 months ago

@datamweb, once your PR is accepted, how do I rerun the tests on my PRs? I know I could do that by adding more commits to PR, but I do not want to...

datamweb commented 4 months ago

once your PR is accepted, how do I rerun the tests on my PRs? I know I could do that by adding more commits to PR, but I do not want to...

You need to rebase . After merged my PR, you can proceed as follows:

git fetch upstream
git switch develop
git merge upstream/develop
git branch syncMeta-fix.bk syncMeta-fix
git switch syncMeta-fix
git rebase upstream/develop
git push --force-with-lease origin syncMeta-fix
dgvirtual commented 4 months ago

hi @lonnieezell, I applied most changes you suggested, but not through the Github interface; not sure how to proceed from this point (and there are more phpstan errors not related to my changes...)

lonnieezell commented 4 months ago

@dgvirtual If it's in the same branch as this PR was originally created in you can just push the changes up.

Ignore PHPStan. I'll have changes to that in the near future :D

dgvirtual commented 4 months ago

@dgvirtual If it's in the same branch as this PR was originally created in you can just push the changes up.

I did just that - see the second commit. But after going through all buttons to address requested changes I still see "Merging is blocked". (Before pushing the second commit I also did a rebase on current develop branch - maybe that was a mistake?).

lonnieezell commented 4 months ago

Oh - merging was blocked because I had requested changes and hadn't done another review approving it. Just did that. Will merge now. Thanks!

dgvirtual commented 4 months ago

Oh - merging was blocked because I had requested changes and hadn't done another review approving it. Just did that. Will merge now. Thanks!

Great, thank you!