[18.0] web_company_color: Add button to reset colors to default - #3448
Conversation
hbrunn
left a comment
hbrunn
left a comment
There was a problem hiding this comment.
note you should call your PRs [$version] $module: $something, and also follow the Odoo git guidelines for the commit message
javiermatos
changed the title
34e8906 to
2247c95
Compare
|
Thank you for your feedback @hbrunn , and for the link to the Odoo git guidelines. I have applied your changes and a few more that were required after it. I have written a better commit message for my last commit but not sure how to fix the rest. |
hbrunn
left a comment
hbrunn
left a comment
There was a problem hiding this comment.
thanks for the followup
not sure how to fix the rest.
what do yo mean here? You should squash your commits anyways, and you can rewrite the commit message while doing so: https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
| def write(self, values): | ||
| if not self.env.context.get("ignore_company_color", False): | ||
| fields_to_check = ( | ||
| "company_colors", |
There was a problem hiding this comment.
haha, I had already my changes from #3484 in mind when suggesting my change. Those two PRs will conflict now, if you could review that one to get it merged, you can rebase your code and remove this part
javiermatos
changed the title
f573867 to
8f71e10
Compare
|
I think it is done @hbrunn . How can we continue? |
hbrunn
left a comment
hbrunn
left a comment
There was a problem hiding this comment.
now you need another positive review and then this can be merged. you might want to review other people's work meanwhile for reciprocity.
site note: usually, you wouldn't touch the translation files and have the translation teams take care of that. If that's generated bullshit, it's much worse than nothing.
|
What a great day to merge this nice PR. Let's do it! |
|
This PR has the |
|
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-3448-by-pedrobaeza-bump-minor. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
/ocabot merge minor |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 18.0-ocabot-merge-pr-3448-by-pedrobaeza-bump-minor. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
Please rebase over latest branch to see the possible problem. |
8f71e10 to
c1a23c5
Compare
c1a23c5 to
d7def44
Compare
|
/ocabot merge minor |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at c787229. Thanks a lot for contributing to OCA. ❤️ |
4 participants
Implements a reset to default button for web_company_color, as once a color is set, the UI does not let you reset it in order to keep using the original Odoo colors.