[17.0][FIX] spreadsheet_oca: add raiseError function to SpreadsheetRenderer#86
Conversation
5e12fd7 to
7df71ee
Compare
|
@chrisandrewmann @pedrobaeza no matter what I do, pre-commit fails in the CI, all green on my local, I have built and rebuilt pre-commit several times. Any chances CI is just crazy for some reason? |
|
I used the output of the failed pre-commit CI job to generate a patch and applied it to the repository, but I’m not entirely at peace with it. For safety, I put it in a separate commit so it can be reviewed or reverted independently. |
I'd ask the opinion of others as i'm not an expert on pre-commit. |
|
Most of them were warnings, not errors. It would be good to have that output gathered in the commit message for knowing which is solving. |
I'm no sure to get what you mean. Do you want this trace in the commit description or the patch I built from it or ...? |
|
@rrebollo I can assure you that those changes done by your pre-commit are not needed. Something weird is in your side that are formatting files other way. Please rollback that and let the GitHub CI to mark the failures. |
|
@pedrobaeza the problem is after the first commit in this PR my local pre-commit passed all green. The trace I just posted is an extract from OCA github CI/CD. So if I rollback the pre-commit test will simply not pass here. Would that be ok for you? From time to time I have my issues with local pre-commit but not often. I work with doodba-copier-template (from your company) as base project. Linux Mint 22.2 (ubuntu noble based). pre-commit through pipx over python 3.11.8 with nvm 24. |
|
It's not possible that all that changes are not included previously in the repository. Something is not working right if so... Please restrict the PR to the change you want to do and let's see the CI error. |
|
@pedrobaeza I reverted the last commit, so now you can see the failing pre-commit test although local pre-commit:
|
|
I still see 24 files changes, so it doesn't seem correct. |
|
I think the mistake is in the other commit. |
188e2bd to
022ff50
Compare
spreadsheet_oca/static/src/spreadsheet/bundle/spreadsheet_renderer.esm.js
Show resolved
Hide resolved
137b659 to
87c0136
Compare
|
The changes have been pushed to this PR. Workaround & Local Hook StatusI am still facing failures with the local pre-commit hooks.
I am continuing to investigate the underlying cause of the local hook failures. I will let you know once I have found a solution. Thanks for your guidance and patience. |
|
Following up on my previous update: Configuration Context FindingI find it noteworthy that the majority of the OCA repositories in branch
Reference example of the missing file: I am continuing to investigate the cause of the local hook failures. |
chrisandrewmann
left a comment
There was a problem hiding this comment.
Looks good to me now
|
@etobella do you consider this bugfix correct? In a first sight, it seems a bit weird to provide on own error class. |
spreadsheet_oca/static/src/spreadsheet/bundle/spreadsheet_renderer.esm.js
Outdated
Show resolved
Hide resolved
87c0136 to
42ecd6a
Compare
|
Please fw-port it to 18 if applicable. /ocabot merge patch |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at ea7b874. Thanks a lot for contributing to OCA. ❤️ |



Fixes #85 and also deal with some pre-commit complaints.
@BinhexTeam HT14857