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

re-export IGeocodeParams (until v2.0.0) #468

Merged
merged 2 commits into from
Feb 26, 2019
Merged

re-export IGeocodeParams (until v2.0.0) #468

merged 2 commits into from
Feb 26, 2019

Conversation

jgravois
Copy link
Contributor

@jgravois jgravois commented Feb 26, 2019

@skitterm aptly pointed out that i removed IGeocodeParams in v1.17.0.

fb2ba9a#diff-094efe5f622403522c0543b08434e397

just because we weren't actually utilizing the interface in any of the geocoding methods doesn't mean other developers weren't using it.

whoops!

this PR just adds a note to the CHANGELOG as breadcrumb. while i was in there i bumped the copyright year and added keywords to our package.json files to fix this 😢

screenshot 2019-02-25 16 21 18

@jgravois jgravois changed the title mention removal of IGeocodeParams mention removal of IGeocodeParams in CHANGELOG Feb 26, 2019
@jgravois
Copy link
Contributor Author

if @tomwayson (or any other SemVer police) tell me i need to bump to v2.0.0, i am not afraid.

@patrickarlt
Copy link
Contributor

@jgravois why not just add it back in until 2.0.0 THEN remove it?

@codecov
Copy link

codecov bot commented Feb 26, 2019

Codecov Report

Merging #468 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #468   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          94     94           
  Lines        1209   1209           
  Branches      219    219           
=====================================
  Hits         1209   1209
Impacted Files Coverage Δ
packages/arcgis-rest-geocoder/src/geocode.ts 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba893b7...01a1170. Read the comment docs.

@tomwayson
Copy link
Member

i am not afraid

I don't do semver for types, so LGTM. Types aren't real anyway.

@patrickarlt
Copy link
Contributor

Types aren't real anyway.
@tomwayson

You monster 👹 👿 !

@jgravois
Copy link
Contributor Author

jgravois commented Feb 26, 2019

why not just add it back in until 2.0.0 THEN remove it?

that never even occurred to me. great idea!

@jgravois jgravois changed the title mention removal of IGeocodeParams in CHANGELOG re-export IGeocodeParams (until v2.0.0) Feb 26, 2019
@patrickarlt
Copy link
Contributor

@jgravois should we really remove this anyway?? leaving it in doesn't bump out bundle sizes and leaving it in also allow TypeScript users to get autocomplete and stricter type checking in their IDEs.

@jgravois
Copy link
Contributor Author

jgravois commented Feb 26, 2019

should we really remove this anyway??

it should absolutely be removed. its an abandoned antipattern.

as you said in #137 (comment), we want folks to extend IRequestOptions, not IParams.

@patrickarlt
Copy link
Contributor

Derp good point.

@jgravois jgravois merged commit 5730e2c into master Feb 26, 2019
@jgravois jgravois mentioned this pull request Feb 28, 2019
35 tasks
@jgravois jgravois deleted the chore/keywords branch April 24, 2019 18:27
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.

3 participants