Use standalone ArrowTypes package #212
Conversation
omus
force-pushed
the
cv/non-relative-ArrowTypes
branch
from
0ea2423
to
30c89ca
Compare
Codecov Report
@@ Coverage Diff @@
## main #212 +/- ##
==========================================
- Coverage 86.08% 83.11% -2.97%
==========================================
Files 26 26
Lines 3025 3092 +67
==========================================
- Hits 2604 2570 -34
- Misses 421 522 +101
Continue to review full report at Codecov.
|
|
As 1.3 seems to be failing on the Manifest.toml I'll switch this over to using the other idea |
|
Yes, I'm in favor of not committing the manifest file |
omus
force-pushed
the
cv/non-relative-ArrowTypes
branch
from
30c89ca
to
8155723
Compare
|
Expecting CI to fail until #213 is merged and registered |
omus
force-pushed
the
cv/non-relative-ArrowTypes
branch
2 times, most recently
from
fbbd33c
to
2905ae3
Compare
| @@ -17,6 +18,7 @@ TimeZones = "f269a46b-ccf7-5d73-abea-4c690281aa53" | |||
| UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4" | |||
|
|
|||
| [compat] | |||
| ArrowTypes = "1.1" | |||
There was a problem hiding this comment.
The Arrow.jl package should only requires ArrowTypes.jl 1.0 but the tests require 1.1.
|
@quinnj please approve the CI to run when you get the chance. There may be a little debugging to do on the CI side before this is ready |
|
I don't have an option to approve CI; that would seem to indicate something in the ci.yml file that's causing CI not to run |
If I edit it in the GitHub editor, it has syntax highlighting and shows errors:
|
Using 1.1 should only be required for running the tests.
omus
force-pushed
the
cv/non-relative-ArrowTypes
branch
from
d128552
to
28d077e
Compare
|
RTM. CI is now functional |
Co-authored-by: Eric Hanson <5846501+ericphanson@users.noreply.github.com>
Should just be the registration. ArrowTypes version 1.1.0 has already been registered. Once this change is merged and registered any package which only depends on ArrowTypes.jl should be able to extend Arrow.jl without having to use Arrow.jl as a dependency. |
|
I'm just validating that adding Arrow support through ArrowTypes.jl is fully working |
We're good here. My validation brought up some things I need to work through which are independent from these changes. |
Fixes: #209.
The one big hangup with this change is the inclusion of the
Manifest.toml. The manifest allows use to devsrc/ArrowTypeswhich means any changes to that code will immediately be reflected during local development. Additionally, since the manifest is committed the CI should also work the same way. Unfortunately, this can lead to changes to ArrowTypes.jl going unregistered (like what has currently happened).Alternatively, we may want to not commit the Manifest.toml file and instead document how to setup a manifest file for local development and never commit it. This approach is more setup for developers but ensures that the CI is testing what users of the packages will ultimately end up seeing. I'm in favor of this approach