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

release 1.14.10 #5709

Closed
jangorecki opened this issue Oct 29, 2023 · 40 comments
Closed

release 1.14.10 #5709

jangorecki opened this issue Oct 29, 2023 · 40 comments
Assignees
Labels
Milestone

Comments

@jangorecki
Copy link
Member

jangorecki commented Oct 29, 2023

We are getting errors on CRAN due to changes in R-devel, therefore we should provide patched version for the current moment, rather then trying to push current master. Release of current master can as well follow the patch release, but releasing master should not be blocking patch release. There is still at least 1 pending issue to resolve in master before it will be ready for CRAN.

Ideally it will be the last release before moving to new governance structure (ultimately that depends on breaking changes in R-devel or breaking changes in CRAN policies).

Issues on a 1.14.9 (1.14.10) milestone should be merged to hotfix1.14.10 branch, and later on to master as well.

@jangorecki jangorecki added this to the 1.14.9 milestone Oct 29, 2023
@jangorecki
Copy link
Member Author

1.14.8...hotfix1.14.10
this will be useful before release to ensure all items have been covered

@TysonStanley
Copy link
Member

Thanks, will get to this over the weekend. Appreciate you setting it up. So the changes for is.atomic() and the suppression of warnings is the only changes I'm seeing, which appear to be what is needed based on the CRAN tests.

A few questions since we are just starting this new phase of having me release to CRAN:

  1. Assume we want to do minor updates to NEWS.md?
  2. Should I make updates to DESCRIPTION to add myself as the maintainer to ensure all contact about the submission comes to me?
  3. Any additional checks that you recommend (e.g. reverse dependencies) be run in addition to the CI processes already in place? This is such a small release that it may not be necessary, but thought I'd get your thoughts on it.

@jangorecki
Copy link
Member Author

There few more things, all related are on the 1.14.9 milestone.

  1. yes, mention in news file will be needed
  2. yes
  3. ideally would be to go through release checklist, even for setting up and understanding the process. It will now be much easier than later on with 1.15.0 because very few things changed so should go very smoothly.

@TysonStanley
Copy link
Member

Sounds good. I'll do those and review the release checklist in depth to start to get comfortable with it.

I agree, it's very convenient we get to do a "practice" round with this much smaller release in advance of the larger 1.15 release.

@tdhock
Copy link
Member

tdhock commented Nov 3, 2023

I don't think revdep checks are necessary for patch releases like this.

@jangorecki
Copy link
Member Author

I pushed NEWS file. It will still need minor amendment when closing #5695

@TysonStanley
Copy link
Member

Updated the dsc file in hotfix1.14.10

@TysonStanley
Copy link
Member

@jangorecki looks like we are waiting on a fix for shift before we submit the patch?

@jangorecki
Copy link
Member Author

Yes, only shift fix is left and we are good for CRAN submission. At the very last moment it is worth to look at CRAN check results, in case another breaking change in R-devel will suddenly popup, so we can still put the fix at the last moment.

@TysonStanley
Copy link
Member

I haven't seen any activity on shift() yet. I'm still waiting on that before submission.

@jangorecki
Copy link
Member Author

Shift was addressed by Michael

@TysonStanley
Copy link
Member

Thanks, will set up the CRAN submission this weekend.

@mmaechler
Copy link
Contributor

mmaechler commented Nov 20, 2023

Thanks, will set up the CRAN submission this weekend.

and then it can happen?
It does look a bit embarrassing to have ERROR on the CRAN check pages for such a long duration..

@TysonStanley
Copy link
Member

TysonStanley commented Nov 26, 2023

While we are working on the final piece for the patch release, was running all checks (both R CMD check and the items in CRAN_release.cmd). For R CMD check, there are a few notes.

  1. The change in maintainer (not a surprise and should be there)
  2. Two URLs connected to https://www.seanlahman.com/ are not working
  3. HTML checks were not happy. For the HTML checks, here is the output.
* checking HTML version of manual ... NOTE
Found the following HTML validation problems:
IDateTime.html:4:1 (IDateTime.Rd:47): Warning: <link> inserting "type" attribute
IDateTime.html:12:1 (IDateTime.Rd:47): Warning: <script> proprietary attribute "onload"
IDateTime.html:12:1 (IDateTime.Rd:47): Warning: <script> inserting "type" attribute
IDateTime.html:17:1 (IDateTime.Rd:47): Warning: <table> lacks "summary" attribute
IDateTime.html:80:1 (IDateTime.Rd:88): Warning: <table> lacks "summary" attribute
J.html:4:1 (J.Rd:5): Warning: <link> inserting "type" attribute
J.html:12:1 (J.Rd:5): Warning: <script> proprietary attribute "onload"
J.html:12:1 (J.Rd:5): Warning: <script> inserting "type" attribute
J.html:17:1 (J.Rd:5): Warning: <table> lacks "summary" attribute
J.html:41:1 (J.Rd:21): Warning: <table> lacks "summary" attribute
[TRUNCATED]
update_dev_pkg.html:4:1 (update_dev_pkg.Rd:3): Warning: <link> inserting "type" attribute
update_dev_pkg.html:12:1 (update_dev_pkg.Rd:3): Warning: <script> proprietary attribute "onload"
update_dev_pkg.html:12:1 (update_dev_pkg.Rd:3): Warning: <script> inserting "type" attribute
update_dev_pkg.html:17:1 (update_dev_pkg.Rd:3): Warning: <table> lacks "summary" attribute
update_dev_pkg.html:37:1 (update_dev_pkg.Rd:12): Warning: <table> lacks "summary" attribute

The other checks produced a few things for us to take note of, but aren't really necessary for the patch release. We can take care of those in the 1.15.0 release.

Note, these checks took place on a Mac M2 running Ventura.

@jangorecki
Copy link
Member Author

jangorecki commented Nov 26, 2023

If you are detecting new problem on your machine, be sure to confirm them on another one, as it may ended up being local configuration issue. Usually docker is good place to reproduce, other can be VM, rhub builder, or CI machines.

You can as well open new issue and ask if anyone else is able to reproduce.

Before going through all checks in release checklist it may be good to resolve last pending items on 1.14.10 milestone as they may affect the outcome of steps in release checklist.

@TysonStanley
Copy link
Member

I agree. This was mostly to get familiar with the process and wanted to bring up any that may impact that release that didn't look connected to the complex issue. Will try to reproduce and if so, will open an issue.

@TysonStanley
Copy link
Member

Not reproducing the HTML notes so can ignore for now.

@jangorecki jangorecki pinned this issue Dec 6, 2023
@jangorecki
Copy link
Member Author

jangorecki commented Dec 6, 2023

#5777 is not being refreshed since 28 Nov so we cannot be sure if it is false positive. We will have to proceed to CRAN submission having this one unresolved for the moment.

@jangorecki

This comment was marked as resolved.

@jangorecki
Copy link
Member Author

Two more rounds of compilation warnings fixes.
We are good to go for CRAN submission.

@TysonStanley
Copy link
Member

Note on Winbuilder for both R-base and R-devel:

Found the following (possibly) invalid URLs:
  URL: https://codecov.io/github/Rdatatable/data.table?branch=master (moved to https://app.codecov.io/github/Rdatatable/data.table?branch=master)
    From: README.md
    Status: 301
    Message: Moved Permanently
  URL: https://github.com/leeper/rio (moved to https://github.com/gesistsa/rio)
    From: NEWS.md
    Status: 301
    Message: Moved Permanently
  URL: https://h2o.ai/blog/behind-the-scenes-of-cran/ (moved to https://h2o.ai/blog/2016/behind-the-scenes-of-cran/)
    From: NEWS.md
    Status: 301
    Message: Moved Permanently
  URL: https://travis-ci.org/Rdatatable/data.table (moved to https://app.travis-ci.com/Rdatatable/data.table)
    From: README.md
    Status: 301
    Message: Moved Permanently
  URL: https://www.posit.co/resources/videos/bigger-data-with-ease-using-apache-arrow/ (moved to https://posit.co/resources/videos/bigger-data-with-ease-using-apache-arrow/)
    From: NEWS.md
    Status: 301
    Message: Moved Permanently
  For content that is 'Moved Permanently', please change http to https,
  add trailing slashes, or replace the old by the new URL.

Looks like another reason to shorten the NEWS.md file. I can update these in hot fix and submit the package.

@jangorecki
Copy link
Member Author

Afair to reproduce those locally xml2 pkg is needed. Described in release procedure.

@TysonStanley
Copy link
Member

Was hoping this was a false positive on Winbuilder bc it was only showing up there but in the other CRAN checks it showed up on Debian too. Any ideas?

Flavor: r-devel-linux-x86_64-debian-gcc, r-devel-windows-x86_64
Check: CRAN incoming feasibility, Result: NOTE
  Maintainer: 'Tyson Barrett <[email protected]>'
  
  New maintainer:
    Tyson Barrett <[email protected]>
  Old maintainer(s):
    Matt Dowle <[email protected]>
  
  Size of tarball: 5211907 bytes

Flavor: r-devel-linux-x86_64-debian-gcc, r-devel-windows-x86_64
Check: S3 generic/method consistency, Result: NOTE
  Apparent methods for exported generics not registered:
    as.IDate.IDate
  See section 'Registering S3 methods' in the 'Writing R Extensions'
  manual.

Flavor: r-devel-linux-x86_64-debian-gcc
Check: re-building of vignette outputs, Result: NOTE
  Re-building vignettes had CPU time 3.2 times elapsed time

Obviously the first isn't an issue and my guess the vignette one is not a surprise or new. But the second one, any ideas on why that one would only pop up on R-devel for Linux/Windows?

@MichaelChirico
Copy link
Member

it looks accurate, maybe it's a new check:

as.IDate.IDate = function(x, ...) x

@jangorecki
Copy link
Member Author

jangorecki commented Dec 7, 2023

  • size of tarballs

Did you compress tests.Rraw as described in release procedure?

  • IDate needs entry in NAMESPACE file
S3method(as.IDate, IDate)
  • Re-building vignettes had CPU time 3.2 times elapsed time

needs setDTthreads(1) in first {r, echo = FALSE, message = FALSE} chunk of each Rmd after attaching data.table namespace

@TysonStanley
Copy link
Member

Thanks.

  • Yes, compressed the .Rraw files but can check that those are what are being built with.
  • Can add the method to NAMESPACE (was just surprised it only showed up on R-devel...)
  • Do the vignettes not have that already in there for a purpose? (ie should I PR that or just use it for the submission)?

@ben-schwen
Copy link
Member

  • Can add the method to NAMESPACE (was just surprised it only showed up on R-devel...)

This is note 2 from 1.13.4. Check news but we could try at least

@jangorecki
Copy link
Member Author

I am also puzzled by this IDate check NOTE.

Tried (on r 4.3.2) _R_CHECK_S3_METHODS_NOT_REGISTERED_=true but no luck.
Found wch/r-source@9e9c839

If anyone will find a switch for that please let me know so I will add in CI.

@tdhock
Copy link
Member

tdhock commented Dec 7, 2023

Maybe this change is too big for the patch/hotfix, but instead of using setDTthreads(1) to fix The "Re-building vignettes had CPU time ..." check, we could get the 2.5 ratio from the CRAN environment variable _R_CHECK_EXAMPLE_TIMING_CPU_TO_ELAPSED_THRESHOLD_ so and then round down as a default for number of threads, as I proposed here #5807

@TysonStanley
Copy link
Member

@tdhock would that still guarantee we'd avoid that note? I assume it would but thought I'd ask. For the patch release I'll do 1 thread but I think we should consider this for the 1.15

@jangorecki
Copy link
Member Author

jangorecki commented Dec 7, 2023

CRAN checks on that are unreliable. There are reports from users that set limit to 2 but still had those notes (for examples, not vignettes). As long as CRAN does not provide reliable way to respect their policies, and as long as these are only vignettes/examples and not users code, then I feel it is better to set it to 1. Just be sure to re-set it at the end. To not change the state of environment that renders vignette.

@TysonStanley
Copy link
Member

Is the best way to reset it by just running setDTthreads()?

@jangorecki
Copy link
Member Author

.old.th = setDTthreads(1)

...

setDTthreads(.old.th)

@jangorecki
Copy link
Member Author

jangorecki commented Dec 8, 2023

Please include those fixes in hotfix branch here, so branch reflects the version submitted to CRAN. Except for the version bump explained in the procedure.

@mmaechler
Copy link
Contributor

it looks accurate, maybe it's a new check:

Yes, it's very new; I've seen it in a package of mine which has been unchanged for years.

@eddelbuettel
Copy link
Contributor

Congrats to all, and especially the new maintainer !!, on getting a new version onto CRAN! On to the 1.15.* series then?

@MichaelChirico
Copy link
Member

indeed! trust dirk to be one of the first to spot the release :)

@jangorecki jangorecki unpinned this issue Dec 8, 2023
@jangorecki
Copy link
Member Author

jangorecki commented Dec 8, 2023

@TysonStanley still please do tag 1.14.10 and push to git, as explained in release procedure, avoiding even version number in the package code. Only in git tag name we want to have even number.

It used to be happen couple times that after CRAN submission new errors has been detected in CRAN checks, that haven't been detected during submission. Then another patch release is needed (slide 9 from here shows that a bit https://jangorecki.gitlab.io/r-talks/2018-07-03_Wroclaw_What_s-new-in-data.table/What-s-new-in-data.table.pdf).
Then we just fork from 1.14.10 tag, add fixed and push as 1.14.12.

When all jobs here https://cran.r-project.org/web/checks/check_results_data.table.html will be OK / NOTE on 1.14.10 then it means we are good. Windows jobs may have false positive failure after examples. It is marked as FAIL rather than ERROR.

@TysonStanley
Copy link
Member

Will do. Probably in the next few hours will be able to get to this.

@TysonStanley
Copy link
Member

Tagged the 1.14.10 release to CRAN with edits that passed CRAN checks (as far as have been run thus far)

https://github.com/Rdatatable/data.table/releases/tag/1.14.10

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

No branches or pull requests

7 participants