r-dbi / RPostgres

A DBI-compliant interface to PostgreSQL
https://rpostgres.r-dbi.org
Other
328 stars 78 forks source link

chore: Vendor cpp11 headers #451

Open pachadotdev opened 8 months ago

pachadotdev commented 8 months ago

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

aviator-app[bot] commented 8 months ago

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes. Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This pull request is currently open (not queued).

How to merge

To merge this PR, comment /aviator merge or add the mergequeue label.


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.
pachadotdev commented 8 months ago

ok @krlmlr , it should look good now

pachadotdev commented 8 months ago

Thanks, looks great!

I do wonder why cpp11::cpp_vendor() insists on putting the headers into inst/include .

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)

krlmlr commented 8 months ago

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.

pachadotdev commented 8 months ago

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 Androidhttps://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 GitHubhttps://github.com/r-dbi/RPostgres/pull/451#issuecomment-1803176710, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACM7UOJVWX4X64FEF7R5BOTYDRRM3AVCNFSM6AAAAAA7DJZF3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBTGE3TMNZRGA. You are receiving this because you authored the thread.Message ID: @.***>

pachadotdev commented 8 months ago

@krlmlr hi, do you think this can be merged?

pachadotdev commented 6 months ago

@krlmlr any update on this? happy holidays

krlmlr commented 3 months ago

Let's wait for https://github.com/r-lib/cpp11/pull/353. Also, the upside of no vendoring means that there's less maintenance effort at the (fairly low) risk of breakages from upstream.