Skip to content

Conversation

@remytms
Copy link
Contributor

@remytms remytms commented Apr 9, 2019

This migrates the module dbfilter_from_header to 12.0.

There is a refactor that was proposed to 11.0: #1340.

Should we wait before the refactor is merge into 11.0 or can we migrate this module to 12.0 and than migrate the changes from 11.0 to 12.0 ?

@remytms remytms mentioned this pull request Apr 9, 2019
32 tasks
@pedrobaeza pedrobaeza added this to the 12.0 milestone Apr 9, 2019
@houssine78
Copy link

👍 I've tested it. It works fine on v12

Copy link
Member

@tbaden tbaden left a comment

Choose a reason for hiding this comment

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

thanks for the work 👍
I did a code review, just some small changes are needed (see below)

],
"data": [
],
"js": [
Copy link
Member

Choose a reason for hiding this comment

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

if you dont use the tag, just delete it.

if config.get('proxy_mode') and \
'dbfilter_from_header' in config.get('server_wide_modules'):
_logger = logging.getLogger(__name__)
_logger.info('monkey patching http.db_filter')
Copy link
Member

Choose a reason for hiding this comment

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

is it really necessary to log?

Choose a reason for hiding this comment

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

it works this way in the previous versions

@houssine78
Copy link

@remytms could you have a quick look to make the few requested changes ?

@remytms
Copy link
Contributor Author

remytms commented May 27, 2019

Yes, I will fix that as soon as possible. :)

@remytms remytms force-pushed the 12.0-mig-dbfilter_from_header branch from 7f1158d to 981593b Compare June 8, 2019 22:01
@remytms
Copy link
Contributor Author

remytms commented Jun 8, 2019

@tbaden I have split the readme according to maintainer-tools and fixed empty keys in manifest. :)

Copy link
Member

@tbaden tbaden left a comment

Choose a reason for hiding this comment

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

@remytms thanks for the changes, just something minor.

"version": "12.0.1.0.0",
"author": "Therp BV, "
"Odoo Community Association (OCA)",
"license": "AGPL-3",
Copy link
Member

Choose a reason for hiding this comment

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

please add the website tag with the value: https://github.com/OCA/server-tools

@remytms remytms force-pushed the 12.0-mig-dbfilter_from_header branch from 981593b to 317e751 Compare June 9, 2019 20:41
@remytms
Copy link
Contributor Author

remytms commented Jun 9, 2019

@tbaden website tag added. Thanks for your attentive review.

@remytms
Copy link
Contributor Author

remytms commented Jun 10, 2019

@houssine78 Can you approve this PR, if it seams good to you ? :)

@pedrobaeza pedrobaeza force-pushed the 12.0-mig-dbfilter_from_header branch from 317e751 to a183841 Compare July 29, 2019 14:16
@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Rebased to 12.0-ocabot-merge-pr-1558-by-pedrobaeza-bump-no, awaiting test results.

@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 32d5666. Thanks a lot for contributing to OCA. ❤️

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

@OCA-git-bot OCA-git-bot merged commit a183841 into OCA:12.0 Jul 29, 2019
OCA-git-bot added a commit that referenced this pull request Jul 29, 2019
Signed-off-by pedrobaeza
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (16.0)
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.

10 participants