Bug 2368546 - Review Request: R-ggdaynight - Add Day/Night Patterns to 'ggplot2' Plots
Summary: Review Request: R-ggdaynight - Add Day/Night Patterns to 'ggplot2' Plots
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL: https://CRAN.R-project.org/package=%{...
Whiteboard:
Depends On: 2368537
Blocks:
TreeView+ depends on / blocked
 
Reported: 2025-05-26 09:48 UTC by Benson Muite
Modified: 2025-10-01 09:17 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
i.ucar86: fedora-review?


Attachments (Terms of Use)

Description Benson Muite 2025-05-26 09:48:46 UTC
spec: https://fed500.fedorapeople.org/R-ggdaynight.spec
srpm: https://fed500.fedorapeople.org/R-ggdaynight-0.1.3-1.fc41.src.rpm

description:
It provides a custom 'ggplot2' geom to add day/night patterns to plots. It
visually distinguishes daytime and nighttime periods. It is useful for
visualizing data that spans multiple days and for highlighting diurnal
patterns.

fas: fed500

Reproducible: Always

Comment 1 Fedora Review Service 2025-05-26 09:50:24 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/9080173
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2368546-r-ggdaynight/fedora-rawhide-x86_64/09080173-R-ggdaynight/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Iñaki Ucar 2025-10-01 09:17:44 UTC
I see that this depends on vdiffr > decor via Suggests, that I've taken too. So I assume that you are interested on this specific package, and not decor or vdiffr, am I right? If this is the case, I would prefer to drop vdiffr from BR and close reviews for vdiffr and decor. Mass rebuilds when a new version of R is released are painful enough right now, and the R packaging guidelines in their current state don't help. So I would prefer to avoid adding more maintenance burden for packages that are not really needed.

I need to find the time to put together new packaging guidelines that reflect the current status of the stack. But basically:

- We need to stop adding Suggests to BR as much as possible. By definition, they are not required. By adding them, we just create a circular dependency hell. Package testthat is one of the exceptions, because it is required for running the test suite. In this case, it means dropping vdiffr.

- But if I drop something in Suggests that is used in the test suite, won't the checks fail? They shouldn't, because by CRAN policy, packages are required to check if a suggested package is installed before using it. So if checks fail, it is a bug in the package. It should be reported upstream and the specific test removed e.g. via a patch (although it's easier to just insert a skip() call in the proper place with sed).

- But doesn't R check for Suggests? Disable this with "export _R_CHECK_FORCE_SUGGESTS_=0", as I do e.g. in https://iucar.fedorapeople.org/pkgs/R-S7.spec

- But what if something in Suggests is used in vignettes? There's no need to rebuild vignettes. Add "--ignore-vignettes" as I do in the link above.

- It makes no sense to rebuild the manual either. You can drop the LaTeX bits in BR, and add "--no-manual".


Also, please note that the last line in %files is duplicated and should be dropped.


Note You need to log in before you can comment on or make changes to this bug.