Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment See the real-time status of this PR on the Aviator webapp. Use the Aviator Chrome Extension to see the status of your PR within GitHub.
|
|
ok @krlmlr , it should look good now |
krlmlr
left a comment
krlmlr
left a comment
There was a problem hiding this comment.
Thanks, looks great!
I do wonder why cpp11::cpp_vendor() insists on putting the headers into inst/include .
|
|
||
| try(dir.create(file.path(vendor_dir, "cpp11"), recursive = TRUE)) | ||
|
|
||
| for (f in finp) { |
There was a problem hiding this comment.
@krlmlr sorry, I accidentally deleted that part, it is a list of all the headers I added. I will re-add that line
There was a problem hiding this comment.
I'll need to run this script on my machine without errors (and perhaps without unnecessary actions).
| } | ||
|
|
||
| unlink(file.path(vendor_dir, "inst"), recursive = TRUE) | ||
|
|
There was a problem hiding this comment.
Everything below here seems to be a one-time action?
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
I will send then a PR, because it is the default, which forced me to write some code for a 1 time action (otherwise I had to create a video of my clicks) |
|
Thanks. I don't think it's a one-time action, at least in the medium term. If cpp11 gets nice new features, we want to use them. |
|
Yes!
I mean, the script can be re-run many times. For now, it is a one time action, as we do not need to run it every time a commit is made
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Kirill Müller ***@***.***>
Sent: Thursday, November 9, 2023 12:17:01 AM
To: r-dbi/RPostgres ***@***.***>
Cc: Mauricio Andres Vargas Sepulveda ***@***.***>; Author ***@***.***>
Subject: Re: [r-dbi/RPostgres] chore: Vendor cpp11 headers (PR #451)
Thanks. I don't think it's a one-time action, at least in the medium term. If cpp11 gets nice new features, we want to use them.
—
Reply to this email directly, view it on GitHub<#451 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACM7UOJVWX4X64FEF7R5BOTYDRRM3AVCNFSM6AAAAAA7DJZF3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBTGE3TMNZRGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@krlmlr hi, do you think this can be merged? |
|
|
||
| try(dir.create(file.path(vendor_dir, "cpp11"), recursive = TRUE)) | ||
|
|
||
| for (f in finp) { |
There was a problem hiding this comment.
I'll need to run this script on my machine without errors (and perhaps without unnecessary actions).
Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>
|
@krlmlr any update on this? |
|
Let's wait for r-lib/cpp11#353. Also, the upside of no vendoring means that there's less maintenance effort at the (fairly low) risk of breakages from upstream. |
2 participants
@krlmlr here are my changes in relation to #450
in the issue I detail where to find my changes and how I vendored the headers
Closes #450.