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.

Sign up for GitHub

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

Jump to bottom

feat: store compute value on unserialize #1107

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: store compute value on unserialize #1107

wants to merge 2 commits into from

Conversation

ArneGudermann
Copy link
Contributor

resolves #709

@ArneGudermann ArneGudermann added bug(fix) Something isn't working or address a specific issue or vulnerability Priority: Medium This issue may be useful, and needs some attention. labels Mar 15, 2024
@ArneGudermann ArneGudermann added this to the ViUR-core v3.7 milestone Mar 15, 2024
sveneberth
sveneberth requested changes Mar 15, 2024
View reviewed changes
src/viur/core/bones/base.py Outdated Show resolved Hide resolved
src/viur/core/bones/base.py Outdated Show resolved Hide resolved
src/viur/core/bones/base.py Outdated Show resolved Hide resolved
@sveneberth
Copy link
Member

But, this does not work for a computed RelationalBone, because this is not calling .serialize()

@ArneGudermann @sveneberth
Apply suggestions from code review
1264416 
Co-authored-by: Sven Eberth <mail@sveneberth.de>
sveneberth added a commit to viur-framework/viur-shop that referenced this pull request Mar 15, 2024
@sveneberth
feat: Prepare compute bones for viur-framework/viur-core#1107
96b09dc
sveneberth
sveneberth previously approved these changes Mar 19, 2024
View reviewed changes
Copy link
Member

@sveneberth sveneberth left a comment •
edited

Choose a reason for hiding this comment

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

So far this seems to work, I observe this for bone types and then we can do a follow-up fix.

@phorward Can we move this to 3.6?

@phorward
Copy link
Member

@phorward Can we move this to 3.6?

Is this still valid? IMHO we can do it.

@phorward phorward changed the base branch from develop to main May 22, 2024 15:39
@phorward phorward dismissed sveneberth’s stale review May 22, 2024 15:39

The base branch was changed.

@phorward phorward requested a review from sveneberth May 22, 2024 15:39
@phorward phorward added the main label May 22, 2024
@phorward phorward added the viur-meeting Issues to discuss in the next ViUR meeting label May 22, 2024
@phorward phorward removed the viur-meeting Issues to discuss in the next ViUR meeting label May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sveneberth sveneberth sveneberth approved these changes

@phorward phorward Awaiting requested review from phorward

Assignees
No one assigned
Labels
bug(fix) Something isn't working or address a specific issue or vulnerability main Priority: Medium This issue may be useful, and needs some attention.
Projects
Documentation
Status: Todo
Milestone
ViUR-core v3.6
Development

Successfully merging this pull request may close these issues.

Store computed values on unserialize
3 participants
@ArneGudermann @sveneberth @phorward