Skip to content

[17.0][MIG] maintenance_inspection#524

Open
edescalona wants to merge 13 commits into
OCA:17.0from
BinhexTeam:17.0-mig-maintenance_inspection
Open

[17.0][MIG] maintenance_inspection#524
edescalona wants to merge 13 commits into
OCA:17.0from
BinhexTeam:17.0-mig-maintenance_inspection

Conversation

@edescalona

Copy link
Copy Markdown

@edescalona edescalona force-pushed the 17.0-mig-maintenance_inspection branch from 8b7e556 to 1b072dd Compare November 5, 2025 15:49
@edescalona edescalona marked this pull request as ready for review November 5, 2025 15:54
@edescalona

edescalona commented Nov 5, 2025

Copy link
Copy Markdown
Author

Hi, @ypapouin @etobella @mymage, if you could do a review, thank you.

@ypapouin ypapouin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You cannot re-use my PR as is. First, because I believe that action path has been introduced in 18.0, and secondly because Odoo 17.0 is using the legacy Kanban view.
You have to remove these two commits:

  • [MIG] maintenance_inspection: Kanban view updated
  • [IMP] maintenance_plan: Path added to action
image

@edescalona

Copy link
Copy Markdown
Author

ping @ypapouin

@edescalona edescalona requested a review from ypapouin December 23, 2025 20:23

@rrebollo rrebollo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please consider my suggestions. Also, I think the last commit should be squashed.

Comment thread maintenance_inspection/models/maintenance_inspection_line.py
Comment thread maintenance_inspection/models/maintenance_plan.py
Comment thread maintenance_inspection/models/maintenance_request.py Outdated
Comment thread maintenance_inspection/models/maintenance_request.py Outdated
Comment thread maintenance_inspection/models/maintenance_request.py Outdated
Comment thread maintenance_inspection/models/maintenance_request.py Outdated
Comment thread maintenance_inspection/readme/DESCRIPTION.md Outdated
@edescalona

Copy link
Copy Markdown
Author

Please consider my suggestions. Also, I think the last commit should be squashed.

I think it would be better to leave that revert history as is, don't you think?

@rrebollo

rrebollo commented Dec 24, 2025

Copy link
Copy Markdown

I think it would be better to leave that revert history as is, don't you think?

Let's see what others think about it

@edescalona

Copy link
Copy Markdown
Author

I think it would be better to leave that revert history as is, don't you think?

Let's see what others think about it

I have no problem removing it; I just checked different pull requests and there are commits like that, but I'll proceed to remove it. Thanks.

@edescalona edescalona force-pushed the 17.0-mig-maintenance_inspection branch from 7411373 to 43f1104 Compare December 24, 2025 15:01
@edescalona

Copy link
Copy Markdown
Author

ping @rrebollo @etobella @ypapouin

@rrebollo rrebollo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit.

Comment thread maintenance_inspection/models/maintenance_equipment.py Outdated
Comment thread maintenance_inspection/models/maintenance_inspection_line.py Outdated
Comment thread maintenance_inspection/models/maintenance_inspection_line.py Outdated
@edescalona edescalona force-pushed the 17.0-mig-maintenance_inspection branch from 43f1104 to 5375cf9 Compare December 24, 2025 18:03
@etobella

Copy link
Copy Markdown
Member

One silly questions, a lot of the changed done in the migration should be in the 18 migration too, like the "readonly" fields or Command changes.

@ypapouin

ypapouin commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

@edescalona , I previously wrote that you have to remove these two (non-squashed on purpose) commits to be compatible with 17.0:

  • [MIG] maintenance_inspection: Kanban view updated
  • [IMP] maintenance_plan: Path added to action
    But instead of doing it, you have reverted these commits changes directly in your "Migration to 17.0" commit. That's not how it should be done.

@etobella I can replicate code changes on my 18.0 PR since all requests are relevant.

Maybe it's more "clean" to make changes in the 18.0 PR and rebase this PR after that ?

@edescalona

Copy link
Copy Markdown
Author

Hi @ypapouin , well I understand that either way would be fine to make the changes, either with reverts or directly in the migration configuration. I can do it in the most convenient way. @etobella , what do you think about that?

@mymage mymage left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Functional OK

@etobella

Copy link
Copy Markdown
Member

@edescalona if some commits were splitted on purpose so you don't need to import it, please remove them.

@edescalona

Copy link
Copy Markdown
Author

Hi @ypapouin , did you make the suggested changes in this migration to the 18 version? Can you confirm so I can perform the rebase again? Thanks.

@ypapouin

Copy link
Copy Markdown
Contributor

Hi @edescalona, I'm on it right now

@ypapouin

ypapouin commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

@edescalona , PR #521 is ready to rebase this one

Like before drop these commits:

  • [MIG] maintenance_inspection: Kanban view updated
  • [IMP] maintenance_plan: Path added to action

The last edit should be to rename list to tree

@edescalona

Copy link
Copy Markdown
Author

Hi @ypapouin, thanks for letting me know.

@edescalona edescalona force-pushed the 17.0-mig-maintenance_inspection branch from 5375cf9 to 696f4ce Compare January 27, 2026 20:20
@edescalona

Copy link
Copy Markdown
Author

ping @ypapouin @etobella

@github-actions

Copy link
Copy Markdown

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions Bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale PR/Issue without recent activity, it'll be soon closed automatically.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants