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

Attempt at trait documentation #183

Merged
merged 8 commits into from
Feb 9, 2024
Merged

Conversation

kylebarron
Copy link
Member

This is relatively a shot in the dark, and I have no idea how much of this is accurate. I'm trying to implement geozero APIs in geoarrow and it's hard to know what I can count on and what I can't.

Closes #182

Copy link
Member

@nyurik nyurik left a comment

Choose a reason for hiding this comment

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

Love it, thanks!!! I think it could mostly be merged as is. One suggestion that might help the novice users - put more of this info in the trait's overall documentation rather than into individual methods. For example, FeatureProcessor doc should explain that its members will be called in the following order (and document the order, with links to the actual methods). Then you don't actually need most of the per-method documentation, apart from parameter documentation.

@kylebarron kylebarron mentioned this pull request Jan 11, 2024
geozero/src/feature_processor.rs Outdated Show resolved Hide resolved
geozero/src/geometry_processor.rs Outdated Show resolved Hide resolved
geozero/src/geometry_processor.rs Outdated Show resolved Hide resolved
geozero/src/property_processor.rs Outdated Show resolved Hide resolved
@michaelkirk michaelkirk added this pull request to the merge queue Feb 9, 2024
Merged via the queue into georust:main with commit 625f07d Feb 9, 2024
1 check passed
@michaelkirk
Copy link
Member

(As an aside, seems like the merge queue is working 👍)

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.

Clarification of GeomProcessor methods
3 participants