Skip to content

Run R package checks via tic and error only on warnings#785

Merged
hannes merged 5 commits intomasterfrom
r/tic
Jul 28, 2020
Merged

Run R package checks via tic and error only on warnings#785
hannes merged 5 commits intomasterfrom
r/tic

Conversation

@pat-s
Copy link
Contributor

@pat-s pat-s commented Jul 26, 2020

fixes #764

Also

  • Linux and Mac runners use a package cache now
  • A code coverage is run in the after_success stage

@pat-s pat-s requested review from hannes and krlmlr July 26, 2020 13:34
krlmlr
krlmlr reviewed Jul 26, 2020
View reviewed changes
Copy link
Contributor

@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.

This will actually give failing builds when R CMD check fails.

tools/rpkg/build-linux Outdated
Comment on lines +13 to +15
python ../upload-s3.py rstats/src/contrib duckdb_*.tar.gz PACKAGES*
python ../upload-s3.py rstats duckdb_*.tar.gz
python3 ../../scripts/asset-upload.py duckdb_r_src.tar.gz=duckdb_*.tar.gz
Copy link
Contributor

@krlmlr krlmlr Jul 26, 2020

Choose a reason for hiding this comment

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

It seems inconsistent to have this code in build-* but only on Mac and Linux. Should this remain the responsibility of .travis.yml, perhaps as part of a separate script?

@pat-s pat-s requested a review from krlmlr July 27, 2020 09:26
@hannes
Copy link
Member

hannes commented Jul 27, 2020

I see
Non-standard files/directories found at top level: 576 ‘deploy-linux’ ‘deploy-mac’ ‘deploy-win’ ‘duckdb_0.2.0.tgz’ 577
Do those need to be added to the build ignore file or does tic just build in the src dir?

@hannes
Copy link
Member

hannes commented Jul 28, 2020

Looking good, I am just wondering about these rather big blobs in before_install. Is that something verbatim from tic?

@pat-s
Copy link
Contributor Author

pat-s commented Jul 28, 2020

We add ccache to speed up source installations of R package deps. For duckdb it also has the effect that it would decrease the internal compiling time when building the package.
I just see that the ccache part is not cached currently so it is not used.

Let me do a quick check to confirm it would actually do its job if I would enable it correctly.

@hannes
Copy link
Member

hannes commented Jul 28, 2020

Looks pretty good no?

@pat-s
Copy link
Contributor Author

pat-s commented Jul 28, 2020

Yes, on Linux we go down from 12 to 5 mins, on mac from 24 to 12 mins (compared to runtime in master).

No ccache on Windows.

Ready from my side.

@hannes hannes merged commit e7eb715 into master Jul 28, 2020
@pat-s pat-s deleted the r/tic branch July 29, 2020 08:33
hawkfish pushed a commit to hawkfish/duckdb that referenced this pull request Dec 21, 2023
Internal duckdb#785: ICU DatePart Serializers
5e8e10e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hannes hannes Awaiting requested review from hannes

@krlmlr krlmlr Awaiting requested review from krlmlr

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fail builds where R CMD has NOTEs

3 participants

@pat-s @hannes @krlmlr