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

Improve continuous integration + tweak to improve R CMD CHECK log #775

Merged
merged 38 commits into from
Sep 15, 2023

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Sep 15, 2023

Addresses #774

Hi @Nowosad ,

here is a messy PR that rassembles my observations / modifications.

Adds CI GH actions (they fail, but I think it's fine. They can always be disabled if needed. But they really helped me fix some easy problems before the rest of the heavy lifting gets done

Note that the remotes field must be removed prior to CRAN submission. But leave it as long as these packages are not on CRAN (tmaptools and cols4all)

Edit: Some examples and tests are failing, this PR doesn't aim to fix that, but rather help pinpointing this

olivroy and others added 30 commits September 14, 2023 12:05
* test pkgdown config

* test
* Update example of tm_layout to silence R CMD CHECK note.

* Update tm_symbols example to silence R CMD CHECK NOTE

* Fix R CMD CHECK on class

* Add :: for non-base functions.

* Remove :: or ::: usage for internal tmap functions.
…omeone tries to install the development version. To be deleted.
  > # Example to illustrate the type of titles
  > tm_shape(World) +
  + 	tm_polygons(c("income_grp", "economy"), title = c("Legend Title 1", "Legend Title 2")) +
  + 	tm_layout(main.title = "Main Title",
  + 			  main.title.position = "center",
  + 			  main.title.color = "blue",
  + 			  title = c("Title 1", "Title 2"),
  + 			  title.color = "red",
  + 			  panel.labels = c("Panel Label 1", "Panel Label 2"),
  + 			  panel.label.color = "purple",
  + 			  legend.text.color = "brown")
  Deprecated tmap v3 code detected. Code translated to v4
  Warning: The 'title' argument of tm_layout is deprecated as of tmap 4.0. Please use tm_title instead.
  Warning: The 'main.title' argument of tm_layout is deprecated as of tmap 4.0. Please use tm_title instead.
  Error in if (is.ena(l$title)) l$title = paste0(names(v), attr(cls, "units")) :
    the condition has length > 1
  Calls: <Anonymous> ... with -> with.default -> eval -> eval -> apply_scale
Propose an alternative to make it an error.
Copy link
Member

@Nowosad Nowosad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olivroy Great work! Please answer to my three comments...

@mtennekes Please take a look (and merge if you think everything is fine)

DESCRIPTION Outdated Show resolved Hide resolved
R/step2_helper_data.R Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@olivroy
Copy link
Contributor Author

olivroy commented Sep 15, 2023

@Nowosad Thanks you for your comments! Addressed them !

@olivroy
Copy link
Contributor Author

olivroy commented Sep 15, 2023

I also copied the content of NEWS from CRAN and changed it to NEWS.md to make it appear on the package website.

image

@olivroy olivroy changed the title Improve continuous integration Improve continuous integration + tweak to improve R CMD CHECK log Sep 15, 2023
@Robinlovelace
Copy link
Collaborator

Looking good!

@olivroy
Copy link
Contributor Author

olivroy commented Sep 15, 2023

Note: about the failed workflows notifications, that can be personalized in Github probably, for a repo and an organization. I think the notifications are not useful as there are too many things failing for now (possibly by design).

But the logs are useful, to see where and when failures happen in the future. https://github.com/settings/notifications

https://docs.github.com/en/actions/monitoring-and-troubleshooting-workflows/notifications-for-workflow-runs

@mtennekes mtennekes merged commit e013d38 into r-tmap:master Sep 15, 2023
1 of 5 checks passed
@mtennekes
Copy link
Member

Great team work, thx!

@olivroy olivroy mentioned this pull request Sep 21, 2023
5 tasks
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.

5 participants