Skip to content

Conversation

@lorenzwalthert
Copy link
Collaborator

@lorenzwalthert lorenzwalthert commented Mar 18, 2024

First commit to try to reproduce the issue on GitHub Actions to make sure we resolve it. Closes #1180.

@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.16%. Comparing base (20c5b37) to head (80a307c).

❗ Current head 80a307c differs from pull request most recent head 89ea11a. Consider uploading reports for the commit 89ea11a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1181   +/-   ##
=======================================
  Coverage   92.16%   92.16%           
=======================================
  Files          46       46           
  Lines        2654     2654           
=======================================
  Hits         2446     2446           
  Misses        208      208           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IndrajeetPatil IndrajeetPatil self-requested a review March 21, 2024 06:26
Copy link
Collaborator

@IndrajeetPatil IndrajeetPatil left a comment

Choose a reason for hiding this comment

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

@lorenzwalthert Tests are fixed. PTAL.

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Mar 21, 2024

Thought of this solution also. 😄 Then once we all use R 4.4, we can go back and activate the check again... Ok sorry missed that you actually fixed it. Good job!

@lorenzwalthert
Copy link
Collaborator Author

Do we still need upload-results: true? I just undid that but now not sure if that was a good idea.

@IndrajeetPatil
Copy link
Collaborator

I find it useful to have the check results available on CI, esp. for OS that I don't have personal access to (Windows).

@lorenzwalthert
Copy link
Collaborator Author

ok sorry then let's revert. But for my curiosity: You still needed to locally install R devel? Or did you manage to find the hint in the test results?

@lorenzwalthert lorenzwalthert merged commit 634dce1 into main Mar 21, 2024
@lorenzwalthert lorenzwalthert deleted the issue-1180 branch March 21, 2024 15:11
@lorenzwalthert
Copy link
Collaborator Author

Thanks a lot again @IndrajeetPatil.

@IndrajeetPatil
Copy link
Collaborator

Actually, I figured out the problem without downloading the devel version locally because the same issue was also present in one of the tests in lintr 😅

@MichaelChirico
Copy link
Contributor

ok sorry then let's revert. But for my curiosity: You still needed to locally install R devel? Or did you manage to find the hint in the test results?

FWIW, in {data.table} we have an Codespace set up that we can easily log into from anywhere to check the package against r-devel:

https://github.com/Rdatatable/data.table/blob/master/.devcontainer/r-devel-gcc/Dockerfile

@lorenzwalthert
Copy link
Collaborator Author

Thanks @MichaelChirico. Is that a free plan? Maybe worth looking into, after the last 3 maintenance, I am sure the next parser change requiring a {styler} release is just around the corner.

@MichaelChirico
Copy link
Contributor

Hmm I couldn't tell what exactly's free from a quick scan of the codespaces page:

https://github.com/features/codespaces

I get a fairly large amount of free time included with my normal $4/mo GitHub pro subscription. Even barring that, it would be free to run that Docker image locally, the advantage of Codespaces is I can do it in the browser from anywhere.

@lorenzwalthert
Copy link
Collaborator Author

I am fine billing the guys who set up the r-lib org 😜. Hehe no, I don’t know if the pricing works per user or per org.

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.

Failure on R devel

5 participants