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.

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

[17.0][MIG] web_leaflet_lib #383

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Conversation

legalsylvain
Copy link
Contributor

@legalsylvain legalsylvain commented Oct 29, 2024

Trivial migration.

Commit to review : c570d60

legalsylvain and others added 4 commits October 29, 2024 11:24
…aflet_lib, to be used by other modules, like 'web_widget_map'. (see : OCA/web#2953)
- Bump version.
- Remove obsolete hook. (that was usefull during the refactoring done here OCA#380)
@legalsylvain legalsylvain force-pushed the 17.0-mig-web_leaflet_lib branch from c570d60 to 737b05e Compare October 29, 2024 12:01
@legalsylvain
Copy link
Contributor Author

/ocabot migration web_leaflet_lib

@OCA-git-bot
Copy link
Contributor

Sorry @legalsylvain you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@legalsylvain legalsylvain changed the title 17.0 mig web leaflet lib [17.0][MIG] web_leaflet_lib Oct 29, 2024
@weinni2000
Copy link

@legalsylvain:
Did you also already convert web_view_leaflet_map to 17/18?
Because otherwise I would like to contribute to that. There is already a demo project in OWL2 from AJScript
https://github.com/ajscriptmedia/odoo-map-view-type/tree/model-arch-parser
Do you know who is the maintainer of geospatial because I need help on the PR for the base geoengine migration to 18.

@legalsylvain
Copy link
Contributor Author

Hi @weinni2000.

regards.

@yvaucher
Copy link
Member

/ocabot migration web_leaflet_lib

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Nov 12, 2024
@OCA-git-bot
Copy link
Contributor

The migration issue (#349) has not been updated to reference the current pull request because a previous pull request (#381) is not closed.
Perhaps you should check that there is no duplicate work.
CC @drkpkg

@lmignon
Copy link
Contributor

lmignon commented Nov 12, 2024

/ocabot migration web_leaflet_lib

@OCA-git-bot OCA-git-bot mentioned this pull request Nov 12, 2024
4 tasks
@yvaucher
Copy link
Member

@weinni2000 Noone volunteered to migrate web_view_leaflet_map yet, contributions are welcome
If you do so please leave a note in one of those issues:
in 17.0 #349
in 18.0 #379

@legalsylvain
Copy link
Contributor Author

Hi. could we merge this trivial one to unblock : OCA/web#2953

thanks !

@lmignon
Copy link
Contributor

lmignon commented Nov 20, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-383-by-lmignon-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 20, 2024
Signed-off-by lmignon
@OCA-git-bot
Copy link
Contributor

@lmignon your merge command was aborted due to failed check(s), which you can inspect on this commit of 17.0-ocabot-merge-pr-383-by-lmignon-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@legalsylvain
Copy link
Contributor Author

@lmignon could you retry ?
the error is related to an error in a call of an unmocked external API.
see : https://github.com/OCA/geospatial/actions/runs/11928567537/job/33245772449#step:8:100

thanks !

@lmignon
Copy link
Contributor

lmignon commented Nov 20, 2024

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-383-by-lmignon-bump-nobump, awaiting test results.

@lmignon
Copy link
Contributor

lmignon commented Nov 20, 2024

@lmignon could you retry ? the error is related to an error in a call of an unmocked external API. see : https://github.com/OCA/geospatial/actions/runs/11928567537/job/33245772449#step:8:100

thanks !

We should mock the call....

@OCA-git-bot OCA-git-bot merged commit 1bdf7ba into OCA:17.0 Nov 20, 2024
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@legalsylvain
Copy link
Contributor Author

We should mock the call....

Arf. There are pro and cons that has been detailled in another Issue / PR. don't find right now the URL, but As far as I remember : Pro : CI is not failing randomly. Cons : we are not testing if the connection to the API works, but we are testing if an emulation of the API works, and API could have changed.
I have no clear point of view on that topic.

Note :

  • the error is on geoengine_base_geolocalize module that you are maintaining.
  • If I understand correctly, base_geolocalize core module is also related to that error. Not sure how we can mook a core test and we should do it.

Anyway, thanks for the merge !

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

Successfully merging this pull request may close these issues.

5 participants