Replace skeleton mapper with orm for Projects #592
Conversation
lib/DataSources/Projects.php
Outdated
There was a problem hiding this comment.
This is the code from lib/DataBuilder/ProjectDataBuilder.php
There was a problem hiding this comment.
A lot was taken from lib/Hydrators/ProjectHydrator.php
lib/Model/Project.php
Outdated
| * @param string[] $keywords | ||
| */ | ||
| public function __construct( | ||
| #[ORM\OneToOne(targetEntity: ProjectStats::class, fetch: 'EAGER')] |
There was a problem hiding this comment.
I chose EAGER in all entities because every piece of data is always used in the website build and I didn't experienced a decrease in speed. Maybe the measured build time in GitHub workflows can be compared with the ones from develop from earlier PRs (if needed).
SenseException
force-pushed
the
projects-orm
branch
from
869c12b
to
dbf34aa
Compare
Empty arrays are stored as null
Once hydrators are gone, this ignored error can be removed again
SenseException
force-pushed
the
projects-orm
branch
from
dbf34aa
to
2daaf03
Compare
| return; | ||
| } | ||
|
|
||
| self::markTestSkipped('This test requires ./bin/console build-website to have been ran.'); |
There was a problem hiding this comment.
| self::markTestSkipped('This test requires ./bin/console build-website to have been ran.'); | |
| self::markTestSkipped('This test requires ./bin/console build-website to have been run.'); |
There was a problem hiding this comment.
I've also fixed the other occurrence where I copied the code from
|
|
||
| $this->projectGitSyncer->expects(self::any()) | ||
| ->method('checkoutDefaultBranch') | ||
| ->with('orm'); |
There was a problem hiding this comment.
Does this expectation actually expect anything? In other words, when could it fail?
There was a problem hiding this comment.
I took the code from lib/DataBuilder/ProjectDataBuilder.php and with it also the old tests. I should review the test and make some useful expectations.
There was a problem hiding this comment.
Commit b05d808e4373c3eb29b6c756d5d1c9a780f78f4a does fix the expected number of calls, but it doesn't appear in the PR 🤔
This is a bigger one to handle.