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

Revamp CI checks #9538

Merged
merged 22 commits into from
Dec 12, 2024
Merged

Revamp CI checks #9538

merged 22 commits into from
Dec 12, 2024

Conversation

dcollins15
Copy link
Contributor

@dcollins15 dcollins15 commented Dec 9, 2024

This PR introduces basic CI checks for Seurat. The implementation uses a Docker-based GitHub Action, nearly identical to the one added for SeuratObject in:

For more details on the underlying Docker image see:

The workflow:

  • Pulls and starts the latest satijalab/seurat-ci image from DockerHub.
  • Installs all dependencies for Seurat.
  • Runs CRAN checks with rcmdcheck (tests are skipped).
  • Runs all tests for Seurat via testthat::test_local

The workflow is currently failing, which is expected. I will follow up soon with fixes for:

❯ checking Rd \usage sections ... WARNING
  Undocumented arguments in Rd file 'SketchData.Rd'
    ‘features’

and

❯ checking R code for possible problems ... [40s/40s] NOTE
  RunSLSI.StdAssay: no visible binding for global variable ‘npcs’
  Undefined global functions or variables:
    npcs

Once the WARNING is resolved I will update the branch rules for develop to require that the Integration Checks: check-package job passes for PRs to be merged. We'll still need to review the output of the check manually to make sure that no problematic NOTES are being thrown.

It looks like the enrichR package was recently removed from CRAN, which is causing the following NOTE:

❯ checking CRAN incoming feasibility ... [10s/73s] NOTE
  Maintainer: ‘Rahul Satija <seurat@nygenome.org>’
  
  Version contains large components (5.1.0.9007)
  
  Suggests or Enhances not in mainstream repositories:
    BPCells, enrichR, presto
  Availability using Additional_repositories specification:
    BPCells   yes   https://bnprks.r-universe.dev/   
    presto    yes   https://satijalab.r-universe.dev/
    enrichR    no   ?

The package’s author is aware of the issue—a fix has been merged—and they’ve indicated that they’ll be re-submitting to CRAN soon. However, until that happens, we'll be blocked from cutting our own release.


This PR also:

  • Applies some housekeeping changes to the package's DESCRIPTION, namely:
    • Adds the and packages as suggested dependencies.
    • Adds the "BuildManual" field and sets its value to true.
    • Sorts all dependencies alphabetically
    • Re-orders the file to group related fields.
  • Incorporates @roi-meir's fix from Fix ssl problem in bioc_install command #9349 to get the AppVeyor pipeline back up and running.
  • Drops files relating to the defunct Travis CI and Azure pipelines—I will remove the relevant webhook shortly.

We'll temporarily leave the AppVeyor pipeline in place so that we can compare their outputs. The new pipeline should be stricter than the old, but we might as well verify this over the next few PRs we merge. Eventually, I'll push a follow-up change dropping the AppVeyor pipeline as well.

I've made similar DESCRIPTION updates for SeuratObject, for more details see:

@dcollins15 dcollins15 changed the base branch from master to develop December 9, 2024 21:49
dcollins15 and others added 7 commits December 9, 2024 19:09
commit a0de550
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 14:22:13 2024 +0300

    Remove env step

commit 9884669
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 14:05:14 2024 +0300

    Move new env under global section

commit ab6cf2d
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 14:04:51 2024 +0300

    Remove R_LIBCURL_SSL_REVOKE_BEST_EFFORT from travis config

commit 53a258d
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 13:56:48 2024 +0300

    Fix CURL_SSL_BACKEND env

commit 215775c
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 13:52:46 2024 +0300

    Try setting CURL_SSL_BACKEND to openssl

commit 680f83c
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 12:36:02 2024 +0300

    Try to see the current env to see if R_LIBCURL_SSL_REVOKE_BEST_EFFORT is supplied correctly

commit 13e055b
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 11:33:40 2024 +0300

    Remove test travis step

commit 92d5f5f
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 11:20:03 2024 +0300

    Check that appveyor is using the branch configuration

commit e3c193f
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 11:13:52 2024 +0300

    Set R_LIBCURL_SSL_REVOKE_BEST_EFFORT in travis.yml

commit 710bab4
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 11:08:24 2024 +0300

    Move R_LIBCURL_SSL_REVOKE_BEST_EFFORT environment variable outside of global section

commit b1d8d51
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 10:35:42 2024 +0300

    Set R_LIBCURL_SSL_REVOKE_BEST_EFFORT to TRUE in string

commit 2701bc2
Author: roi-meir <meir.roi@gmail.com>
Date:   Mon Sep 30 10:13:31 2024 +0300

    SET R_LIBCURL_SSL_REVOKE_BEST_EFFORT to TRUE in appveyor script

    Deal with ssl problem in bioc_install command
@dcollins15 dcollins15 merged commit 7754874 into develop Dec 12, 2024
2 of 3 checks passed
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.

3 participants