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

add R examples and geoarrow #108

Merged
merged 4 commits into from
Oct 24, 2022
Merged

Conversation

yeelauren
Copy link
Contributor

Added geoarrow package for R in readme and a minimal example with plotting.

@cholmes
Copy link
Member

cholmes commented May 26, 2022

Awesome! Excited to add this. It looks like you had a markdown formatter going so this PR has a lot more changes than just the couple of new lines. Could you make it so it's just a PR on the new lines?

I don't think we're opposed to markdown formatting improvements, but it'd be best to put them in their own, clean PR so the changes are clear. I do think we should move towards some automated formatting or at least lint checking for markdown, so it is all consistent.

@kylebarron
Copy link
Collaborator

probably good to cc @paleolimbot directly too for 👍

@paleolimbot
Copy link
Collaborator

The example looks great! Thank you for adding!

I agree about the Markdown formatting...it's considered bad git karma but when this happens to me I usually just copy/paste the original back from GitHub, find my changes from the Changed Files tab on the PR, and copy/paste those back into the document (with apologies if you already know all about this!).

@yeelauren
Copy link
Contributor Author

Thanks for the tips, actually one of my first PR's so appreciate it. Have some automatic linting going on on save. I hope this last commit fixes it ?

@cholmes
Copy link
Member

cholmes commented Oct 24, 2022

Sorry for the delay on this, but looks great - thanks for your contribution @yeelauren!

@cholmes cholmes merged commit d00ad62 into opengeospatial:main Oct 24, 2022
@cholmes
Copy link
Member

cholmes commented Oct 24, 2022

Ugh, I merged this but it looks like it lead to a lint failure - I apologize. I have to run right now, but I'll try to investigate soon. It'd be good to run our linter on branches and not merge if it's failing, so we hit the errors before merging instead of after.

@kylebarron
Copy link
Collaborator

Lint is already set up to run on every pull request:

On the first commit on this PR lint failed, but then not sure why no later commits show that CI was run at all

@kylebarron kylebarron mentioned this pull request Oct 25, 2022
@jorisvandenbossche
Copy link
Collaborator

This also makes me wonder: do we somehow want to keep track for each implementation we list which version they support?
Because I see that geoarrow is still at 0.3.0, and doesn't yet write 0.4.0 files.

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

5 participants