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

Upgrade to OpenLayers v5 #631

Merged
merged 35 commits into from
Sep 6, 2018
Merged

Upgrade to OpenLayers v5 #631

merged 35 commits into from
Sep 6, 2018

Conversation

marcjansen
Copy link
Member

@marcjansen marcjansen commented May 7, 2018

REFACTORING

Description:

This supersedes #606 (this is a rebased version for which the build passes.)

Let's stay tuned and be compatible with prospective avaliable upgraded OpenLayers v5.x! 🤸‍♂️

This PR contains first steps to get married with newest ol which is currently avaliable as v5.0.0-beta.10.

Most important changes:

  • all ol classes imports are in camelcase now
  • for the following classes the named imports for affected functions are used now:
    • ol/easing
    • ol/proj
    • ol/proj/proj4
    • ol/extent
    • ol/interaction/Draw
    • ol/events/Condition
    • ol/Observable
    • ol/sphere
    • ol/format/Filter

Upgrade considers notes mentioned here and here.

Enjoy! 🍍

Fixes #515

@marcjansen
Copy link
Member Author

The build currently fails on Node 10, but passes locally for me:

image

This needs more investigation.

@dnlkoch
Copy link
Member

dnlkoch commented May 23, 2018

I recognized the failing test in another PR as well and I think it's not related to the changes made here. Dealing with setTimeout() with a fixed timeout is always tricky…

@dnlkoch
Copy link
Member

dnlkoch commented May 24, 2018

Apparently #667 fixes the failing tests. It's almost the same change as introduced here (thanks @marcjansen), but it also overhauls the tests in the CircleMenuItem spec.

@annarieger
Copy link
Member

annarieger commented Jul 4, 2018

ol5 (v5.0.0) has been released 8 days ago 👏
I'll try to tackle still existing conflicts tonight and prepare this PR for merging..

@marcjansen
Copy link
Member Author

I'll try to tackle still existing conflicts tonight

That is nice, thanks.

and prepare this PR for merging.

Before merging we should release. The first release which contains this PR shold IMO be a major release with clear advice what is needed in projects to make use of that new release.

Thanks.

@annarieger
Copy link
Member

I totally agree with all your points @marcjansen. That's why I wrote "prepare for merging".. This doesn't mean that I'm gonna do it directly 😉

@annarieger annarieger force-pushed the ol5-upgrade branch 3 times, most recently from c80bbc9 to a76a54c Compare July 5, 2018 09:21
@annarieger annarieger changed the title [WIP] Upgrade to OpenLayers v5 Upgrade to OpenLayers v5 Jul 5, 2018
@annarieger
Copy link
Member

PR is ready for review now. It's rebased to include all significant changes from master branch and supports currently latest avaliable release v5.0.2 of ol.

As @marcjansen mentioned above, we should mandatory release react-geo due to breaking changes in the API before this one is merged.

@KaiVolland
Copy link
Member

Is there a general update strategy? New features only to for OL5?

@marcjansen
Copy link
Member Author

marcjansen commented Jul 5, 2018

Is there a general update strategy? New features only to for OL5?

@KaiVolland See description:

Upgrade considers notes mentioned here and here.

Let's just release a new minor with the latest changes and then release a new major with this in. The major is justifuied due to changed peer dependency. release notes should include a big marker linking to the above documents.

@KaiVolland
Copy link
Member

Sry my question wasn't precise enough.
I wanted to know how react-geo should handle new features of react-geo. So will we continue to support v8 (ol4) or will we just add new features to >v9 (ol5). Bugs should be fixed for both i think.

Copy link
Member

@ahennr ahennr left a comment

Choose a reason for hiding this comment

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

Nice work, plz merge

@KaiVolland KaiVolland merged commit c54dc12 into master Sep 6, 2018
@marcjansen marcjansen deleted the ol5-upgrade branch September 6, 2018 15:53
hblitza pushed a commit that referenced this pull request Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants