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

Changes 6: Version class improvements and unit tests #6450

Merged
merged 25 commits into from
Jun 12, 2024

Conversation

bastianallgeier
Copy link
Member

@bastianallgeier bastianallgeier commented May 17, 2024

This PR …

Refactoring

  • Move more logic from ContentStorage to Version where it makes sense
    • Version::contentFile
    • Modified Version::delete to always delete all languages
    • Version::touchLanguage
  • Pass Language objects to storage methods
  • Return null in ::modified if a version does not exist.
  • Add unit tests

Outlook

Breaking changes

None

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and checks all pass

For review team

  • Add changes & docs to release notes draft in Notion (not relevant as the Version class is added in v5 alpha 1)

@bastianallgeier bastianallgeier added this to the 5.0.0-alpha.1 milestone May 17, 2024
@bastianallgeier bastianallgeier force-pushed the v5/changes/version-class-improvements branch 3 times, most recently from 5cea85b to 00105f6 Compare May 21, 2024 11:31
@bastianallgeier bastianallgeier force-pushed the v5/changes/version-class-improvements branch from 77c290e to c336cd3 Compare May 22, 2024 12:32
@bastianallgeier bastianallgeier changed the base branch from v5/develop to v5/changes/content-storage-handler-abstract May 22, 2024 12:33
@bastianallgeier bastianallgeier force-pushed the v5/changes/content-storage-handler-abstract branch from f3528f1 to 53629c8 Compare June 5, 2024 09:05
@bastianallgeier bastianallgeier force-pushed the v5/changes/version-class-improvements branch from c336cd3 to dd746cb Compare June 5, 2024 09:07
Base automatically changed from v5/changes/content-storage-handler-abstract to v5/develop June 10, 2024 19:30
@lukasbestle lukasbestle marked this pull request as ready for review June 10, 2024 19:30
src/Content/Version.php Outdated Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
src/Content/Version.php Outdated Show resolved Hide resolved
src/Content/Version.php Outdated Show resolved Hide resolved
src/Content/Version.php Outdated Show resolved Hide resolved
tests/Content/VersionTest.php Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
src/Content/Version.php Show resolved Hide resolved
tests/Content/VersionTest.php Show resolved Hide resolved
@bastianallgeier
Copy link
Member Author

@lukasbestle I think we can ignore the coverage. Codecov is considering the Language::single line uncovered although it clearly is covered by multiple tests.

@lukasbestle
Copy link
Member

It was covered, but not marked as covered 😉

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.

3 participants