search

python-visualization / folium

Python Data. Leaflet.js Maps.
https://python-visualization.github.io/folium/
MIT License
6.93k stars 2.23k forks source link

Remove the deadlink in `CONTRIBUTING.md` which mentions manual installation of chromedriver and Chrome #1891

Closed achieveordie closed 8 months ago

achieveordie commented 8 months ago

The current step 7 in Contributing Code subsection of CONTRIBUTING.md mentions this link which has since been 404ed. The correct link should be pointing towards https://sites.google.com/chromium.org/driver/ which mentions using CfT dashboard to download the apt. version.

However, even that is not required since the inception of Selenium Manager which automatically downloads and caches the required drivers.

The actionable step would be to either completely remove this step or mention all these details for those interested. Happy to update the docs depending on the choice core devs make :)

Edit: Forgot to mention that I've tried running the selenium tests locally and they pass without any manual installations.

Conengmo commented 8 months ago

Hi @achieveordie, thanks for pointing this out. If you want to update our contributing guide with a better approach to running the Selenium tests locally, that's very welcome! I'm not really up to date with developments in that topic, so your help is appreciated.

The actionable step would be to either completely remove this step or mention all these details for those interested.

I'm not really sure what's needed or not. But in general I'd like the contributing guide to be as simple as possible. What's the minimal thing needed to be able to run our tests? Hope that helps in deciding what should be in there and what not.

achieveordie commented 8 months ago

What's the minimal thing needed to be able to run our tests?

In that case, we can remove this step. Installing selenium (version 4.11 and above for Chrome) will be enough to run the tests. I encourage you to try it out, perhaps on a VM. This should simplify setting up folium for new contributors.

Conengmo commented 8 months ago

Thanks again @achieveordie for making this issue!