Skip to content

Conversation

@BT-pgaiser
Copy link

@BT-pgaiser BT-pgaiser commented Nov 17, 2025

Due to the introduction of the ThrowAwayCache, in full log mode when attempting to log a write, the reading of the 'new' values effectively reads the 'old' values because prior changes were not flushed to the database yet at that point. This resulted in missing log lines in the created log entry since the old vs. new value comparison did not indicate any changes.

It has been confirmed that without the introduced patch for the database flush, the added test fails as expected.

@BT-pgaiser
Copy link
Author

Tackles issues #3444 and #3420.

@BT-pgaiser
Copy link
Author

@StefanRijnhart Requesting a review.

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Very nice, thank you! Just a nitpick regarding the flushing.

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks!

@StefanRijnhart
Copy link
Member

Oh, please squash commits into one.

Due to the introduction of the ThrowAwayCache, in full log mode when attempting
to log a write, the reading of the 'new' values effectively reads the 'old'
values because prior changes were not flushed to the database yet at
that point. This resulted in missing log lines in the created log entry since
the old vs. new value comparison did not indicate any changes.
@BT-pgaiser
Copy link
Author

@StefanRijnhart Can we please merge this if it's OK from your side? Thanks.

@StefanRijnhart
Copy link
Member

We cannot merge this yet, see section 6 of https://odoo-community.org/resources/code.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@StefanRijnhart
Copy link
Member

Dear BT members, please consider to not pile up these inter-team reviews as you did here. It feels like you are playing the system. It would be different if the reviews come from known and appreciated contributors to the OCA but I see that for all of you the review on this PR is the only OCA review you did in the whole month (or, ever).

@BT-rmartin
Copy link

BT-rmartin commented Nov 28, 2025

@StefanRijnhart

According to the rules...
Your code will be merged upon 3 positive reviews within 5 days (or 2 after more than 5 days). At least one of the review above must be from a member of the PSC or someone from the OCA Core Maintainer. Find more info here.

So max 2 reviews can be done by whoever is not PSC or OCA Core Maintainer, so I guess here is the problem that we did approve it 4 times.

I want to clarify our position. We don't just simply "approve", we do take the time to properly review the code, and we all are OCA members, although perhaps not with huge activity.
In particular @BT-anieto has contributed with several fixes or migrations of modules
https://github.com/pulls?q=is%3Apr+author%3ABT-anieto+archived%3Afalse+user%3AOCA+is%3Aclosed
Also @BT-tkarpinski did a migration and a fix contributed to OCA
https://github.com/pulls?q=is%3Apr+author%3ABT-tkarpinski+archived%3Afalse+is%3Aclosed+user%3AOCA
In the case of @BT-ojossen he has also some contributions with new modules, fixes or translations https://github.com/pulls?q=is%3Apr+author%3ABT-ojossen+archived%3Afalse+is%3Aclosed+user%3AOCA+
Finally, @BT-ssteiner also did a migration and an improvement in the past https://github.com/pulls?q=is%3Apr+author%3ABT-ssteiner+archived%3Afalse+is%3Aclosed+user%3AOCA+

Our organization is not "OCA-first", but we do use OCA modules and contribute mainly with migrations, improvements and fixes from time to time, as well as reviews of the contributions of our colleagues.

My question is: Is it a problem that we contribute even if our contributions are not monthly or very frequent?
I would expect that any contributio, whether code, reviews, or support, is welcome.

So, what is the expectation from the OCA regarding reviewers?
Should we limit ourselves to one reviewer per company?
Is it a requirement to review PRs from other contributors as well?

We want to do it better and be more aligned with community expectations.
Thanks in advance for your insights.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants