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

Add R CMD Check workflow back for duckdb src PRs #8845

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

Tmonster
Copy link
Contributor

@Tmonster Tmonster commented Sep 8, 2023

The duckdb R package is no longer in the duckdb source. We still want to check with every PR that the duckdb R package can built with no errors. This PR will fix that by running the r-cmd-check on duckdb PRs. Tests are not run, only building the R package is checked

the workflow is as follows

  1. checkout duckdb pull request branch
  2. checkout duckdb-r source
  3. Setup R
  4. Install duckdb package dependencies
  5. update the duckdb-r source with the source from the pull request branch (using the ./vendor.sh script)
  6. Run R CMD build . and R CMD check .
  7. sanity check that the library can be imported.

@Tmonster
Copy link
Contributor Author

Tmonster commented Sep 8, 2023

Any notes about the platforms that this runs on? The check also takes a while >20 min, so I'm not sure if we want to run this on 3 platforms for every PR.

Copy link
Collaborator

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Yeah, I'm also working on shortening the build times in duckdb-r.

shell: bash
working-directory: ${{ env.DUCKDB_R_SRC }}
run: ./vendor.sh

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can enable parallel builds:

      - name: Build in parallel
        run: |
          mkdir -p ~/.R
          echo 'MAKEFLAGS = -j2' >> ~/.R/Makevars
      shell: bash

@github-actions github-actions bot marked this pull request as draft September 11, 2023 08:27
@Tmonster Tmonster marked this pull request as ready for review September 11, 2023 08:29
@hannes
Copy link
Member

hannes commented Sep 11, 2023

I think one platform (Linux?) is fine here

@github-actions github-actions bot marked this pull request as draft September 11, 2023 11:33
@Tmonster Tmonster marked this pull request as ready for review September 13, 2023 11:00
@hannes
Copy link
Member

hannes commented Sep 13, 2023

@Mytherin any objections?

@Mytherin
Copy link
Collaborator

LGTM

@hannes hannes merged commit 12644f5 into duckdb:main Sep 13, 2023
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.

None yet

4 participants