OCA / search-engine

GNU Affero General Public License v3.0
45 stars 60 forks source link

[16.0][FIX] connector_search_engine: missing typing-extensions external dependency #194

Closed clementmbr closed 4 months ago

clementmbr commented 4 months ago

Trivial PR to fix this kind of bug in CI: https://github.com/OCA/sale-channel/actions/runs/8332403108/job/22801435634?pr=17#step:7:446

This bug doesn't occur on search-engine repo because the module search_engine_serializer_pydantic use pydantic which depends on python package typing-extensions. So typing-extensions is loaded anyway.

But when a module on another repo like sale_channel needs to use connector_search_enginewithout using search_engine_serializer_pydantic, the bug occurs.

Once this PR is merged I will be able to merge https://github.com/OCA/sale-channel/pull/17

cc @florian-dacosta @bealdav @rousseldenis @adrienpeiffer

rousseldenis commented 4 months ago

Thanks for this fix I don't know if the lib has to be added manually in requirement file, because it say it is "generated from manifests external_dependencies".

I am not sure though and I guess it won't hurt.

Pre-commit does the job for you if you add a dependency in manifest

clementmbr commented 4 months ago

I don't know if the lib has to be added manually in requirement file, because it say it is "generated from manifests external_dependencies".

I didn't run pre-commit on my local branch, so I had to add the lib to the requirements manually as the pre-commit test went red here.

Anyway, @OCA/search-engine-maintainers is it possible to merge this trivial PR? Thanks!

rvalyi commented 4 months ago

/ocabot merge patch

OCA-git-bot commented 4 months ago

On my way to merge this fine PR! Prepared branch 16.0-ocabot-merge-pr-194-by-rvalyi-bump-patch, awaiting test results.

OCA-git-bot commented 4 months ago

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