Skip to content

[17.0][MIG] web_save_discard_button#2872

Merged
OCA-git-bot merged 6 commits intoOCA:17.0from
joepsanders:17.0-mig-web_save_discard_button
Jan 7, 2025
Merged

[17.0][MIG] web_save_discard_button#2872
OCA-git-bot merged 6 commits intoOCA:17.0from
joepsanders:17.0-mig-web_save_discard_button

Conversation

@joepsanders
Copy link
Copy Markdown
Contributor

@joepsanders joepsanders commented Jul 4, 2024

No description provided.

@legalsylvain
Copy link
Copy Markdown
Contributor

legalsylvain commented Jul 4, 2024

/ocabot migration web_save_discard_button

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Jul 4, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 4, 2024
Open
33 tasks
@joepsanders joepsanders force-pushed the 17.0-mig-web_save_discard_button branch 2 times, most recently from db02e3e to 24aa8cc Compare July 4, 2024 20:06
@joepsanders
Copy link
Copy Markdown
Contributor Author

joepsanders commented Jul 12, 2024

@legalsylvain this module was migrated, codecov checks fail on existing code. (How) should I fix this?

@legalsylvain
Copy link
Copy Markdown
Contributor

legalsylvain commented Jul 12, 2024

codecov checks fail on existing code.

No. V17 branch has a coverage of 95% (https://github.com/OCA/web/tree/17.0)
your patch has a coverage of 77% (https://app.codecov.io/gh/OCA/web/pull/2872?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=OCA)

If a PR propose code less covered, Codecov CI is red.

Once said, it's just 2 lines. If you think it's relevant to write a test on that topic, please do it. IMO, it's OK like this.

kind regards.

@joepsanders
Copy link
Copy Markdown
Contributor Author

joepsanders commented Jul 12, 2024

IMO, it's OK like this.

I agree, so how does this proceed?

sanderlienaerts
sanderlienaerts approved these changes Aug 2, 2024
View reviewed changes
Copy link
Copy Markdown

@sanderlienaerts sanderlienaerts left a comment

Choose a reason for hiding this comment

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

LGTM

@legalsylvain
Copy link
Copy Markdown
Contributor

legalsylvain commented Aug 2, 2024

Hi @joepsanders : could you take a look if #2899 (review) is still present, and try to fix it if not ?

web_responsive should be installed.

thanks !

sanderlienaerts
sanderlienaerts reviewed Aug 5, 2024
View reviewed changes
Comment thread web_save_discard_button/__manifest__.py Outdated
"maintainers": ["synconics"],
"depends": ["web"],
"data": [],
"images": ["static/description/main_screen.png"],
Copy link
Copy Markdown

@sanderlienaerts sanderlienaerts Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This deprecated line may be removed. There are also some redundant lines due to being a default value already (https://www.odoo.com/documentation/17.0/developer/reference/backend/module.html).

Copy link
Copy Markdown
Contributor Author

@joepsanders joepsanders Aug 5, 2024

Choose a reason for hiding this comment

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

Thanks, fixed the manifest. Please check :)

@sanderlienaerts
Copy link
Copy Markdown

sanderlienaerts commented Aug 5, 2024

Hi @joepsanders : could you take a look if #2899 (review) is still present, and try to fix it if not ?

web_responsive should be installed.

thanks !

Tested in 17.0 with web_save_discard_button and web_responsive installed, the bug presented there is not present.

@joepsanders joepsanders force-pushed the 17.0-mig-web_save_discard_button branch from 24aa8cc to cc8b9b3 Compare August 5, 2024 08:57
sanderlienaerts
sanderlienaerts approved these changes Aug 7, 2024
View reviewed changes
Copy link
Copy Markdown

@sanderlienaerts sanderlienaerts left a comment

Choose a reason for hiding this comment

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

LGTM

@sanderlienaerts
Copy link
Copy Markdown

sanderlienaerts commented Sep 24, 2024

@legalsylvain can we push this forward?

@legalsylvain
Copy link
Copy Markdown
Contributor

legalsylvain commented Sep 24, 2024

two review are required for merging !
feel free to review some PRs, to ask people to review your PR.

thanks !

@legalsylvain
Copy link
Copy Markdown
Contributor

legalsylvain commented Jan 7, 2025

/ocabot merge patch

No review.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

OCA-git-bot commented Jan 7, 2025

On my way to merge this fine PR!
Prepared branch 17.0-ocabot-merge-pr-2872-by-legalsylvain-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit dfcf5ee into OCA:17.0 Jan 7, 2025
@OCA-git-bot
Copy link
Copy Markdown
Contributor

OCA-git-bot commented Jan 7, 2025

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

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

Reviewers

2 more reviewers

@sanderlienaerts sanderlienaerts sanderlienaerts approved these changes

@mtbochaca mtbochaca mtbochaca approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved merged 🎉

Projects

None yet

Milestone

17.0

Development

Successfully merging this pull request may close these issues.

8 participants

@joepsanders @legalsylvain @sanderlienaerts @OCA-git-bot @mtbochaca @sam-synconics @Ivorra78 @mymage