Skip to content

Conversation

@schulzh
Copy link
Contributor

@schulzh schulzh commented Feb 11, 2025

This fixes two issues we've encountered with the manifest serialization.

The first is that docker was not serialized as a deep structure, but as a string (java object representation), which breaks the yaml. The second issue is that the disk quota was not serialized with the correct field name.

@anthonydahanne
Copy link
Contributor

Hello @schulzh , thank you for your contribution!
Could you please add a unit test just to assert this does not change inadvertently?

@schulzh
Copy link
Contributor Author

schulzh commented Feb 16, 2025

Hi @anthonydahanne , I've added a unit test, could you take a look again?

@schulzh schulzh changed the title fix docker serialization and disk_quota fix docker, disk_quota and readiness check type serialization Feb 21, 2025
@schulzh schulzh changed the title fix docker, disk_quota and readiness check type serialization fix docker, disk_quota and health+readiness check type serialization Feb 21, 2025
@schulzh
Copy link
Contributor Author

schulzh commented Feb 21, 2025

Hi @anthonydahanne , I've added two more serialization fixes to the PR.

@Kehrlann Kehrlann self-requested a review April 30, 2025 07:46
Copy link
Contributor

@Lokowandtg Lokowandtg left a comment

Choose a reason for hiding this comment

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

lgtm
(but not tested)

@Kehrlann Kehrlann self-assigned this May 28, 2025
@Kehrlann Kehrlann added this to the 6.0.0 release milestone May 28, 2025
@Kehrlann
Copy link
Contributor

Hi @schulzh ! Thanks for your contribution.

Would you mind rebasing on main before we can merge this please?

@schulzh
Copy link
Contributor Author

schulzh commented Jun 10, 2025

Hi @Kehrlann , i've updated the branch.

Copy link
Contributor

@Kehrlann Kehrlann left a comment

Choose a reason for hiding this comment

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

Looks good!
I was hoping for a rebase rather than a merge, but that's OK.

@Kehrlann Kehrlann merged commit e18f771 into cloudfoundry:main Jun 10, 2025
5 checks passed
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.

4 participants