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

Fix tests to prepare for upcoming leaflet v2.2.0 release #60

Merged
merged 1 commit into from Aug 15, 2023
Merged

Fix tests to prepare for upcoming leaflet v2.2.0 release #60

merged 1 commit into from Aug 15, 2023

Conversation

gadenbuie
Copy link
Contributor

Hi @trafficonese! We're preparing the next release of leaflet (v2.2.0, rstudio/leaflet#876) and noticed that our updates break a test in your package.

I've provided a fix in this PR. In essence, leaflet() now includes its own dependencies in the dependencies item of the returned htmlwidgets object, which means that it contains both our dependencies and the dependencies your extension has added. The updated test now finds the name of all dependencies in the object and ensures that lfx-movingmarker and leaflet-awesomemarkers are included. This ensures that they're included in the list of dependencies, but you may want to add an additional check to test that they're included in that order (if that's important to your extension).

Do you think you'd be able to update your package on CRAN in the next two weeks? We're planning on submitting leaflet v2.2.0 on Tuesday, August 29 at the latest and are hoping we can have all reverse dependencies issues worked out by then. Thanks!

@gadenbuie gadenbuie mentioned this pull request Aug 15, 2023
6 tasks
@trafficonese
Copy link
Owner

Thanks, I think thats fine and the order should not matter.
I'll update the package in the next days!

@trafficonese trafficonese merged commit 0a606be into trafficonese:master Aug 15, 2023
7 of 8 checks passed
@gadenbuie gadenbuie deleted the fix-for-leaflet-v2-2 branch August 16, 2023 20:30
@trafficonese
Copy link
Owner

its on cran!

@gadenbuie
Copy link
Contributor Author

Thanks @trafficonese!

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

Successfully merging this pull request may close these issues.

None yet

2 participants