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

[15.0][MIG] mig mail_show_follower #862

Merged
merged 11 commits into from
Mar 16, 2022
Merged

[15.0][MIG] mig mail_show_follower #862

merged 11 commits into from
Mar 16, 2022

Conversation

Shide
Copy link
Contributor

@Shide Shide commented Mar 11, 2022 •
edited
Loading

Hello,
this is a straight foward migration from v12, so minor changes has been made.

  • @api.multi removed
  • super() simplified
  • Bumped version

@rafaelbn @pedrobaeza @ValentinVinagre please review if you want :)

MT-230 @moduon

ValentinVinagre and others added 10 commits March 11, 2022 12:10
@ValentinVinagre @Shide
ADD mail_show_follower copied from https://github.com/sygel-technolog…
ea1424a 
…y/sy-server-backend/tree/12.0/mail_show_follower
@eLBati @Shide
IMP mail_show_follower metadata and style
87329ae 
Display CC according to notification_type
@oca-travis @Shide
[UPD] Update mail_show_follower.pot
fc1494b
@OCA-git-bot @Shide
[UPD] README.rst
962aef2
@OCA-git-bot @Shide
[ADD] icon.png
ee10005
@ypapouin @Shide
Translated using Weblate (French)
c7a1b4f 
Currently translated at 100.0% (7 of 7 strings)

Translation: social-12.0/social-12.0-mail_show_follower
Translate-URL: https://translation.odoo-community.org/projects/social-12-0/social-12-0-mail_show_follower/fr/
@eLBati @Shide
FIX mail_show_follower: ValueError: Expected singleton: res.users
e7eeb2c
@rafaelbn @Shide
[12.0][mail_show_follower] Update version
a271a65 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
@rafaelbn @MiquelRForgeFlow @Shide
Update mail_show_follower/__manifest__.py
ee07cb3 
Co-authored-by: Miquel Raïch <miquel.raich@eficent.com>
@Shide
[IMP] mail_show_follower: black, isort, prettier
f75e5ef
@rafaelbn
Copy link
Member

/ocabot migration mail_show_follower

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Mar 11, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 11, 2022
Closed
36 tasks
@rafaelbn
Copy link
Member

FYI @cvinh after this PR is reviewed and merged we will backport it to v14

@flotho
Copy link
Member

flotho commented Mar 11, 2022

many tests are failing,

@Shide
Copy link
Contributor Author

Shide commented Mar 11, 2022

many tests are failing,

pre-commit are failing because in this lines https://github.com/OCA/social/pull/862/files#diff-b44d3d8a58eb40149e86dfa92e460ba5597bdab00d92d09f71c1d56c76a6444fR40-R44, black is doing the auto-split lines due the long of it. I've tryied to fix it but always auto-split the line and puts the "and" after the sentence.
Then flake8 is warning that the "and" is on the next line (the auto-split of black)

codecov is warning that it's not tested, but I'm doing a straight foward migration.

runboat is failing because it's trying to locate rsync, but it is not part of the migration.

@flotho
Copy link
Member

flotho commented Mar 11, 2022

many tests are failing,

pre-commit are failing because in this lines https://github.com/OCA/social/pull/862/files#diff-b44d3d8a58eb40149e86dfa92e460ba5597bdab00d92d09f71c1d56c76a6444fR40-R44, black is doing the auto-split lines due the long of it. I've tryied to fix it but always auto-split the line and puts the "and" after the sentence. Then flake8 is warning that the "and" is on the next line (the auto-split of black)

codecov is warning that it's not tested, but I'm doing a straight foward migration.

runboat is failing because it's trying to locate rsync, but it is not part of the migration.

Hi @Shide , already had this one.
This was mainly because of spaces at the start of the new line.

@Shide Shide force-pushed the 15.0-mig-mail_show_follower branch from 0b1a4a1 to ecaf6d7 Compare March 14, 2022 08:10
@rafaelbn rafaelbn requested a review from yajo March 14, 2022 18:57
@Shide
Copy link
Contributor Author

Shide commented Mar 15, 2022

many tests are failing,

pre-commit are failing because in this lines https://github.com/OCA/social/pull/862/files#diff-b44d3d8a58eb40149e86dfa92e460ba5597bdab00d92d09f71c1d56c76a6444fR40-R44, black is doing the auto-split lines due the long of it. I've tryied to fix it but always auto-split the line and puts the "and" after the sentence. Then flake8 is warning that the "and" is on the next line (the auto-split of black)
codecov is warning that it's not tested, but I'm doing a straight foward migration.
runboat is failing because it's trying to locate rsync, but it is not part of the migration.

Hi @Shide , already had this one. This was mainly because of spaces at the start of the new line.

@flotho Done, thanks! :)

This was referenced Mar 15, 2022
Open
Closed
yajo
yajo approved these changes Mar 16, 2022
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Code review

@rafaelbn
Copy link
Member

/ocabot merge nobump

@ValentinVinagre @HaraldPanten we can improve this module as you want and when you what with your improvements of https://github.com/sygel-technology/sy-server-backend/tree/14.0/mail_show_follower

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 15.0-ocabot-merge-pr-862-by-rafaelbn-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 57b51a1 into OCA:15.0 Mar 16, 2022
@OCA-git-bot
Copy link
Contributor

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

@Shide Shide deleted the 15.0-mig-mail_show_follower branch March 16, 2022 09:56
ValentinVinagre
ValentinVinagre reviewed Mar 16, 2022
View reviewed changes
Copy link
Contributor

@ValentinVinagre ValentinVinagre left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@tafaRU
Copy link
Member

tafaRU commented Mar 21, 2022 •
edited
Loading

I have backported the module to 14.0. Could you please take a look at #867?

@Shide
Copy link
Contributor Author

Shide commented Mar 21, 2022 •
edited
Loading

Some customizations are ready to merge. Review if you want: #866

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

@ValentinVinagre ValentinVinagre ValentinVinagre left review comments

@rafaelbn rafaelbn rafaelbn approved these changes

@yajo yajo yajo approved these changes

@HaraldPanten HaraldPanten Awaiting requested review from HaraldPanten

Assignees

@Shide Shide

Labels
approved merged 🎉 needs review
Projects
None yet
Milestone
15.0
Development

Successfully merging this pull request may close these issues.
10 participants
@Shide @rafaelbn @flotho @OCA-git-bot @tafaRU @yajo @ValentinVinagre @eLBati @oca-travis @ypapouin