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

Enhanced profile for maven enhanced tests #167

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hurelhuyag
Copy link

No description provided.

@hurelhuyag hurelhuyag marked this pull request as ready for review January 12, 2023 10:04
@cowtowncoder
Copy link
Member

Could you please describe what is the goal here; why is the addition needed?

@hurelhuyag
Copy link
Author

@cowtowncoder Compile time enhancement is an important option. Those commits confirmed the project does not work with compile-time enhanced entities. Now I'm trying to understand how this project works. As far as I know, findProxied methods should replaced with Hibernate.isPropertyInitialized and Hibernate.isInitialized methods. I successfully implemented LazyBeanSerializer and LazyBeanUnwrappingSerializer. But It seems not enough. I also needed to customize CollectionSerializer

@cowtowncoder
Copy link
Member

@hurelhuyag Ok, typically all PRs should have descriptions explaining the rationale which is why I asked. So it's to add verification of working (or not as the case is) of compile-time enhanced entities. That makes sense.

I can't really merge this while things are failing as it's not possible to verify regressions.
But maybe this could be defined as a separate workflow? (Github action)

@hurelhuyag
Copy link
Author

hurelhuyag commented Jan 14, 2023

@cowtowncoder I am not sure about a separate workflow. This should work fine.

    - name: Build
      run: ./mvnw -B -q -ff -ntp verify
    - name: Build With Enhancement
      run: ./mvnw -Penhanced -B -q -ff -ntp verify

@cowtowncoder
Copy link
Member

@hurelhuyag Yes but I will not merge changes to CI that would prevent "all green". So verification can only be added to main workflow when it passes. I would be ok with separate workflow that would be failing, just not main failing because that makes it more difficult to determine if PRs cause new breakages.

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.

2 participants