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

Get ready for Leaflet 1.0 #851

Closed
simison opened this issue Jul 15, 2015 · 31 comments
Closed

Get ready for Leaflet 1.0 #851

simison opened this issue Jul 15, 2015 · 31 comments

Comments

@simison
Copy link
Contributor

simison commented Jul 15, 2015

http://leafletjs.com/2015/07/15/leaflet-1.0-beta1-released.html

The release sports a number of minor breaking API changes, and some of the plugins will have to be updated. To make the upgrade less painful, we’re doing a short beta cycle before the final 1.0 to allow users and plugin developers to catch up to the changes while we find and fix remaining issues.

:-)

@simison simison changed the title Get ready for 1.0 Get ready for Leaflet 1.0 Jul 15, 2015
@tombatossals
Copy link
Owner

Wow, great news. That would be a good moment for giving this directive a power up.

Thanks for reporting!

@paambaati
Copy link

@tombatossals Is there a branch for this that we could check out/test/contribute?

@nmccready
Copy link
Contributor

Referencing Mobile Draw Support . IE: This is another reason to get started on this.
Wildhoney/Leaflet.FreeDraw#3

@nmccready
Copy link
Contributor

I will be adding a leaflet 1.0.X branch to start working towards this. However I am not sure what the name of the branch should be.

leaflet-1.0.X to targets leaflet? Or just 1.0.X as we are working towards a 1.0.X ourselves.

My only beef with making it 1.0.X is that going to leaflet 1.0.X might not be stable right away. But keeping the branch with 1.0.X and mapping to leaflet 1.0.X helps eliminate confusion.

Thoughts anyone?

@nmccready nmccready self-assigned this Sep 23, 2015
@nmccready nmccready added this to the 1.0.0 (maybe) milestone Sep 23, 2015
@vrubert
Copy link

vrubert commented Sep 23, 2015

Looks good @nmccready, I think too we need that branch because it will need some time to have the directive working with leaflet 1.0.x and all the modules.

BTW, the Leaflet.FreeDraw project is amazing!!

@nmccready
Copy link
Contributor

@vrubert @tombatossals any thoughts on what you want the branch name to be?

@simison
Copy link
Contributor Author

simison commented Sep 24, 2015

Keeping the future version number of this package at the branch name makes more sense to me rather than following Leaflet versions. Trying to follow Leaflet versions and keeping them in pair with package, might prove hard in long term, doable only maybe for major versions.

I'd call it "1.0.0-with-leaflet-1.x", "1.0.0-WIP" or just "1.0.0"

@tombatossals
Copy link
Owner

Hey, I would suggest "1.0.0-wip" as @simison said, what do you think?

I haven't had time to take a look at leaflet-1.0-beta, I expect there will be no major API changes and 90% will work seamlessly. I hope so.

@nmccready
Copy link
Contributor

Awesome I hope so too, what is WIP? Work In Progress? I am not sure if that is needed as all branches are WIP 😀 .

@nmccready
Copy link
Contributor

Or is it WLP?

@nmccready
Copy link
Contributor

hmm maybe just leaflet-1.X

@nmccready
Copy link
Contributor

That way if we have any major changes (unseen foreseeable) we don't need to rename the branch.

@tombatossals
Copy link
Owner

You're right, all branches are WIP :D

Totally agree, better leaflet-1.X so no need to rename it later.

@nmccready
Copy link
Contributor

cool I'll push the branch up

@simison
Copy link
Contributor Author

simison commented Sep 24, 2015

"WIP" suggests that it's currently in progress and shouldn't be expected to work. ;-)

leaflet-1.x is cool tho. Thanks for working on it!

@nmccready
Copy link
Contributor

Working on this, and I can not find the leaflet-src.js dist file for master or 1.0.0-beta1

@nmccready
Copy link
Contributor

See Leaflet/Leaflet#3635

@nmccready
Copy link
Contributor

Good lord, they made it incompatible with bower.. ie it is not standardized anymore.

Leaflet/Leaflet#1354

@nmccready
Copy link
Contributor

Ok, so far we have a few alternatives

  • bower will point to "leaflet": "http://cdn.leafletjs.com/downloads/leaflet-1.0.0-b1.zip" . The big problem with this is that it makes our library and leaflet dependencies an invalid mess where version number really means nothing.
  • become a npm project and abandon bower
  • fork leaflet to get the tags to have dist/*.js files again

This is a real problem with a lot of future headache. It would be nice if they could start using bower correctly again. But I am not confident this will happen.

@simison
Copy link
Contributor Author

simison commented Sep 30, 2015

Hmm, nope, normally they do have dist files for releases. (Leaflet/Leaflet#3635 (comment))

It's just this beta which is lacking those files (Leaflet/Leaflet#3635 (comment)); it'll be fixed with the next version.

For WIP using the beta-1 zip path is fine.

@nmccready
Copy link
Contributor

Yeah they need to do another release. However this still doesn't make me that confident in bower as a solution. This will probably be a headache in the future for sure. I vote for becoming npm centric.

@nmccready
Copy link
Contributor

This brings up a good discussion though. It would be nice to not always be committing dist/*.js as it would keep the PR size very small and easier to review. I wonder if we should start doing something similar and only publish dist/*.js on tags/releases. Alternatively we could have a dist repo. Ugh.. all more work I don't feel like doing.

@nmccready
Copy link
Contributor

With the dist/*.js ignored solution; this would only mess with people that target master. This would be tons of plunkers and many users.

@nmccready
Copy link
Contributor

All the branch exists on the main repo. Feel free to start making the specs work. On a positive note the 604 example appears to work at least partially.

@simison
Copy link
Contributor Author

simison commented Sep 30, 2015

Bower will eventually die out as NPM evolves, I suppose. Anyway, for this sort of package it's very important to stay distribution channel agnostic and just keep supporting everything that's feasible. Bower is still hugely popular for good reasons.

Having a CI script to produce dist files sounds totally something this package should have. :-) Have a look at Leaflet's build scripts?

@nmccready
Copy link
Contributor

There scripts are all using jake, I would prefer to look at grunt or gulp as I do not feel like learning another tool just for another tool sake.

@nmccready
Copy link
Contributor

If someone wants to take that work on by all means go for it. I am not nominating myself at all for a dist repo. However, I can support bower still if it is desired. But I think we need to better support npm. Also it would be nice to have privileges to push to npm as only @tombatossals has that right now.

@simison
Copy link
Contributor Author

simison commented Sep 30, 2015

+1 for better npm support

@ghost
Copy link

ghost commented Oct 16, 2015

If bower support disappears, then A LOT of mobile ionic apps will stop working, best to support both until one consumes the other.

@nmccready
Copy link
Contributor

It looks like leaflet is back on board with bower for now so we should be ok with both.

@tombatossals
Copy link
Owner

I'm going to rework&redesign angular-leaflet-directive to be compatible with Leaflet v1.0. It will mantain almost all its functionality, and will be compatible with the current features of the directive, but I must start from a fresh point, so I'm going to close this issue. If you think it must be worked with the new version, please reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants