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

[IMP] rename_fields: replaced domain having both style of quotes #295

Merged
merged 1 commit into from
Jul 15, 2022
Merged

[IMP] rename_fields: replaced domain having both style of quotes #295

merged 1 commit into from
Jul 15, 2022

Conversation

royle-vietnam
Copy link
Contributor

No description provided.

@pedrobaeza

This comment was marked as resolved.

Sign in to view
@royle-vietnam

This comment was marked as resolved.

Sign in to view
@pedrobaeza

This comment was marked as resolved.

Sign in to view
@royle-vietnam

This comment was marked as resolved.

Sign in to view
@royle-vietnam royle-vietnam changed the title [IMP] rename_fields: correct syntax on REPLACE() function [IMP] rename_fields: replaced domain having both style of quotes Jul 13, 2022
@royle-vietnam
Copy link
Contributor Author

Please review again, thank you

@MiquelRForgeFlow
Copy link
Contributor

error

@pedrobaeza
Copy link
Member

OK, thanks for the extra explanations. Then this may be applicable to other regexp replaces? And also the possibility of single or double quotes apply to the context replace as well? A final question: is there the possibility of having spaces in between the tuples of the domain?

@pedrobaeza
Copy link
Member

Why having #294 as well instead of everything together?

@royle-vietnam
Copy link
Contributor Author

OK, thanks for the extra explanations. Then this may be applicable to other regexp replaces? And also the possibility of single or double quotes apply to the context replace as well? A final question: is there the possibility of having spaces in between the tuples of the domain?

Maybe, but it doesn't matter, since we're replace for "field" in tuples of the domain

@pedrobaeza
Copy link
Member

OK, let's merge this one for clear the pending things. Please check the rest of the questions on the rest of the code.

@pedrobaeza pedrobaeza merged commit 101323a into OCA:master Jul 15, 2022
@ndd-odoo
Copy link
Contributor

Why having #294 as well instead of everything together?

For my point of view, i prefer to separate things, and isn't it easier for you to review right?

@pedrobaeza
Copy link
Member

Yeah, no problem at all. It was just that for a moment, I didn't know what I was seeing, hehe. And thanks for all this work 😄

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

@pedrobaeza pedrobaeza pedrobaeza left review comments

@ndd-odoo ndd-odoo ndd-odoo approved these changes

@MiquelRForgeFlow MiquelRForgeFlow MiquelRForgeFlow approved these changes

Assignees
No one assigned
Labels
approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@royle-vietnam @pedrobaeza @MiquelRForgeFlow @ndd-odoo @OCA-git-bot