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

Made implicit conversions to/from Raster and ProjectedRaster deprecated. #2834

Merged

Conversation

@metasim
Copy link
Member

commented Dec 1, 2018

Partially addresses #2829.

Made implicit conversions to/from Raster and ProjectedRaster deprecated.
Partially addresses #2829.

Signed-off-by: Simeon H.K. Fitch <fitch@astraea.io>

@metasim metasim requested review from echeipesh and pomadchin Dec 1, 2018

@pomadchin
Copy link
Member

left a comment

LGTM, but I wrote a couple of non critical questions.

implicit def rasterToFeature[T <: CellGrid](r: Raster[T]): PolygonFeature[T] =
r.asFeature

/**
* Implicit conversion from a PolygonFeature to a [[Raster]].
*/
@deprecated("Implicit conversions considered unsafe", "2.1.1")

This comment has been minimized.

Copy link
@pomadchin

pomadchin Dec 2, 2018

Member

I don't have a strong opinion about this deprecation warning. Can't we just remove them as we're preparing 3.0 release and definitely can break API?

This comment has been minimized.

Copy link
@metasim

metasim Dec 3, 2018

Author Member

When you say "remove them" do you mean the @deprecation annotations, or the messages?

This comment has been minimized.

Copy link
@pomadchin

pomadchin Dec 3, 2018

Member

Conversions, we can just mention in docs that we removed these implicits.

This comment has been minimized.

Copy link
@echeipesh

echeipesh Dec 3, 2018

Contributor

Back-porting it and publishing the deprecation warnings with 2.1.1 is defiantly a good-guy move and we should do it.

This comment has been minimized.

Copy link
@metasim

metasim Dec 4, 2018

Author Member

I'm still confused as to what you want me to do with this part.

@metasim

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

NB: Build failed only due to one config timing out.

@pomadchin
Copy link
Member

left a comment

Looks good to me, @metasim do you want to do smth else in terms of this PR?
Also probably it makes sense to create a separate PR just to remove these conversions.

@metasim

This comment has been minimized.

Copy link
Member Author

commented Dec 4, 2018

If you're happy with the PR I say merge it. And yes, a separate PR for removal in 3.0 with appropriate changelog notice.

@pomadchin pomadchin merged commit 42f94a6 into locationtech:master Dec 7, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ip-validation
Details

@pomadchin pomadchin added this to the 3.0 milestone Dec 7, 2018

@metasim metasim deleted the metasim:metasim/deprecated-implicit-raster branch Dec 7, 2018

echeipesh added a commit that referenced this pull request Dec 28, 2018
Merge pull request #2834 from metasim/metasim/deprecated-implicit-raster
Made implicit conversions to/from Raster and ProjectedRaster deprecated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.