Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split core geo types into new geo-types crate. #201

Merged
merged 1 commit into from
May 3, 2018
Merged

Split core geo types into new geo-types crate. #201

merged 1 commit into from
May 3, 2018

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Apr 14, 2018

The idea is to move the core data types into a new, minimal crate called geo-types. The idea here is to have all third party crates (e.g. rust-gdal, rust-geojson) rely on this new crate instead pulling in all the baggage of the geo crate.

This shouldn't be a breaking change since all types in geo-types are reexported by the geo crate.

@urschrei
Copy link
Member

Need to have a closer look, but very much in favour of the idea. This keeps tripping us up in other ecosystem crates.

@jdroenner
Copy link
Member

I like this idea. rust-gdal currently only uses the core types. (Could we also have a geo-traits later?)

@@ -0,0 +1,94 @@
// FIXME: everything in this file is copy/paste from the 'geo' crate. ideally we
// wouldn't have this duplication
Copy link
Member Author

Choose a reason for hiding this comment

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

the (non-public) traits below are unfortunately necessary to keep the spade trait impls on the geo types in geo-types/src/lib.rs. for now it's a little messy, but i think we can reevaluate this in the future. i.e. do we really need spade?

@frewsxcv frewsxcv changed the title Experiment with a new geo-types crate. [feedback requested] Split core geo types into new geo-types crate. Apr 23, 2018
@frewsxcv
Copy link
Member Author

@urschrei @jdroenner Alright, finished working on this. If either of you have some time, would love a pair of eyes 👀 for a review!

@frewsxcv
Copy link
Member Author

frewsxcv commented May 3, 2018

bors r+

bors bot added a commit that referenced this pull request May 3, 2018
201: Split core geo types into new geo-types crate. r=frewsxcv a=frewsxcv

The idea is to move the core data types into a new, minimal crate called `geo-types`. The idea here is to have all third party crates (e.g. `rust-gdal`, `rust-geojson`) rely on this new crate instead pulling in all the baggage of the `geo` crate.

This shouldn't be a breaking change since all types in `geo-types` are reexported by the `geo` crate.

Co-authored-by: Corey Farwell <coreyf@rwell.org>
@urschrei
Copy link
Member

urschrei commented May 3, 2018

👏

@bors
Copy link
Contributor

bors bot commented May 3, 2018

Build succeeded

@bors bors bot merged commit 858fa50 into master May 3, 2018
@frewsxcv frewsxcv deleted the geo-typese branch May 3, 2018 11:20
bors bot added a commit to georust/geojson that referenced this pull request May 4, 2018
93: Switch 'geo' dependency to 'geo-types'. r=frewsxcv a=frewsxcv

georust/geo#201

# before

```
geojson v0.10.0 (file:///Users/corey/dev/rust-geojson)
[dependencies]
├── geo v0.8.2
│   [dependencies]
│   ├── failure v0.1.1
│   │   [dependencies]
│   │   ├── backtrace v0.3.6
│   │   │   [dependencies]
│   │   │   ├── cfg-if v0.1.2
│   │   │   ├── libc v0.2.37
│   │   │   └── rustc-demangle v0.1.7
│   │   └── failure_derive v0.1.1
│   │       [dependencies]
│   │       ├── quote v0.3.15
│   │       ├── syn v0.11.11
│   │       │   [dependencies]
│   │       │   ├── quote v0.3.15 (*)
│   │       │   ├── synom v0.11.3
│   │       │   │   [dependencies]
│   │       │   │   └── unicode-xid v0.0.4
│   │       │   └── unicode-xid v0.0.4 (*)
│   │       └── synstructure v0.6.1
│   │           [dependencies]
│   │           ├── quote v0.3.15 (*)
│   │           └── syn v0.11.11 (*)
│   ├── num-traits v0.2.1
│   ├── serde v1.0.27
│   ├── serde_derive v1.0.27
│   │   [dependencies]
│   │   ├── quote v0.3.15 (*)
│   │   ├── serde_derive_internals v0.19.0
│   │   │   [dependencies]
│   │   │   ├── syn v0.11.11 (*)
│   │   │   └── synom v0.11.3 (*)
│   │   └── syn v0.11.11 (*)
│   └── spade v1.4.0
│       [dependencies]
│       ├── cgmath v0.16.0
│       │   [dependencies]
│       │   ├── approx v0.1.1
│       │   ├── num-traits v0.1.43
│       │   │   [dependencies]
│       │   │   └── num-traits v0.2.1 (*)
│       │   └── rand v0.4.2
│       │       [dependencies]
│       │       └── libc v0.2.37 (*)
│       ├── clamp v0.1.0
│       ├── nalgebra v0.14.0
│       │   [dependencies]
│       │   ├── alga v0.5.2
│       │   │   [dependencies]
│       │   │   ├── approx v0.1.1 (*)
│       │   │   ├── num-complex v0.1.42
│       │   │   │   [dependencies]
│       │   │   │   ├── num-traits v0.2.1 (*)
│       │   │   │   └── rustc-serialize v0.3.24
│       │   │   └── num-traits v0.1.43 (*)
│       │   ├── approx v0.1.1 (*)
│       │   ├── generic-array v0.8.3
│       │   │   [dependencies]
│       │   │   ├── nodrop v0.1.12
│       │   │   └── typenum v1.9.0
│       │   ├── matrixmultiply v0.1.14
│       │   │   [dependencies]
│       │   │   └── rawpointer v0.1.0
│       │   ├── num-complex v0.1.42 (*)
│       │   ├── num-traits v0.1.43 (*)
│       │   ├── rand v0.4.2 (*)
│       │   └── typenum v1.9.0 (*)
│       ├── num v0.1.42
│       │   [dependencies]
│       │   ├── num-bigint v0.1.43
│       │   │   [dependencies]
│       │   │   ├── num-integer v0.1.36
│       │   │   │   [dependencies]
│       │   │   │   └── num-traits v0.2.1 (*)
│       │   │   ├── num-traits v0.2.1 (*)
│       │   │   ├── rand v0.4.2 (*)
│       │   │   └── rustc-serialize v0.3.24 (*)
│       │   │   [dev-dependencies]
│       │   │   └── rand v0.4.2 (*)
│       │   ├── num-complex v0.1.42 (*)
│       │   ├── num-integer v0.1.36 (*)
│       │   ├── num-iter v0.1.35
│       │   │   [dependencies]
│       │   │   ├── num-integer v0.1.36 (*)
│       │   │   └── num-traits v0.2.1 (*)
│       │   ├── num-rational v0.1.42
│       │   │   [dependencies]
│       │   │   ├── num-bigint v0.1.43 (*)
│       │   │   ├── num-integer v0.1.36 (*)
│       │   │   ├── num-traits v0.2.1 (*)
│       │   │   └── rustc-serialize v0.3.24 (*)
│       │   └── num-traits v0.2.1 (*)
│       ├── pdqselect v0.1.0
│       └── smallvec v0.3.3
│       [dev-dependencies]
│       └── cgmath v0.16.0 (*)
├── num-traits v0.2.1 (*)
├── serde v1.0.27 (*)
└── serde_json v1.0.10
    [dependencies]
    ├── dtoa v0.4.2
    ├── itoa v0.3.4
    ├── num-traits v0.2.1 (*)
    └── serde v1.0.27 (*)
```

# after

```
geojson v0.11.0 (file:///Users/corey/dev/rust-geojson)
[dependencies]
├── geo-types v0.1.0
│   [dependencies]
│   └── num-traits v0.2.1
├── num-traits v0.2.1 (*)
├── serde v1.0.27
└── serde_json v1.0.10
    [dependencies]
    ├── dtoa v0.4.2
    ├── itoa v0.3.4
    ├── num-traits v0.2.1 (*)
    └── serde v1.0.27 (*)
```

Co-authored-by: Corey Farwell <coreyf@rwell.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.

None yet

3 participants