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 GeoJSON type constants #42

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

gabplch
Copy link
Contributor

@gabplch gabplch commented Nov 27, 2023

No description provided.

@gabplch gabplch force-pushed the add-consntants-for-geojson-types branch from f1911fe to d7cafa8 Compare November 27, 2023 00:57
src/GeoJson.php Outdated Show resolved Hide resolved
src/GeoJson.php Outdated Show resolved Hide resolved
src/Geometry/LineString.php Outdated Show resolved Hide resolved
tests/GeoJsonTest.php Outdated Show resolved Hide resolved
tests/Geometry/LineStringTest.php Outdated Show resolved Hide resolved
tests/Geometry/LineStringTest.php Outdated Show resolved Hide resolved
tests/Geometry/LineStringTest.php Outdated Show resolved Hide resolved
tests/Geometry/LinearRingTest.php Outdated Show resolved Hide resolved
tests/Geometry/LinearRingTest.php Outdated Show resolved Hide resolved
src/GeoJson.php Outdated Show resolved Hide resolved
@jmikola
Copy link
Owner

jmikola commented Nov 28, 2023

Thanks for the contribution. I'll look into the CI failure (related to my Scrutinizer CI account) when I get a chance and get this PR merged/released.

This would be a good use of enums; however, the install stats still show a significant number of pre-8.1 users so I don't think it'd be worth bumping for that reason alone.

@gabplch
Copy link
Contributor Author

gabplch commented Dec 1, 2023

What about the enums, I don't really think, that it is the place, where is is really needed.

@jmikola
Copy link
Owner

jmikola commented Dec 1, 2023

What about the enums

A defined set of constants would be a fine case for a backed enum; however, I don't think it's worth bumping the language requirement of the package. Happy to keep things as they are in this PR.

@gabplch
Copy link
Contributor Author

gabplch commented Dec 1, 2023

Would you like to add more checks like phpstan/phpcs or others?

And when will you create new release?) I need it on my current project ^)

@jmikola jmikola changed the title add constants Add GeoJSON type constants Dec 3, 2023
@jmikola
Copy link
Owner

jmikola commented Dec 3, 2023

I fixed the Scrutinizer issue. Please remove the order imports commit (3b3d4fb) from this PR and I can proceed with merging. I'm not sure why that was added but I'm not interested in those changes and prefer to keep use statements for classes and functions grouped separately.

@gabplch gabplch force-pushed the add-consntants-for-geojson-types branch from 3b3d4fb to 9f4b255 Compare December 4, 2023 00:06
@gabplch
Copy link
Contributor Author

gabplch commented Dec 4, 2023

done

@jmikola jmikola merged commit 845c70b into jmikola:master Dec 4, 2023
13 checks passed
@jmikola jmikola added this to the 1.2.0 milestone Dec 4, 2023
@jmikola
Copy link
Owner

jmikola commented Jan 17, 2024

Lost track of this after merging but I just tagged 1.2.0, which includes this and #45.

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.

2 participants