TimHeckel / meteor-bootboxjs

A meteor smart package for the bootstrap bootboxjs plugin
30 stars 19 forks source link

Bootstrap-3 is a dependency #13

Closed albertmatyi closed 10 years ago

albertmatyi commented 10 years ago

I don't know the proper solution, but the problem is that some people don't use the bootstrap-3 dependency but the bootstrap3-less, and we wouldn't want to include them both. And obviously bootbox would work with the latter as well.

What I would like to propose is to remove the bootstrap-3 dependency - as we don't have a mechanism to check if either one is already included and include it based on that information.

Any opinions?

mizzao commented 10 years ago

I think that once weak dependencies are properly implemented in the Meteor packaging system, we can add them in for both bootstrap-3 and bootstrap3-less, etc. to ensure that the code for those packages is loaded first if bootbox is included. This should be available soon.

If the dependency is not added, there can be errors that show up during load.

alanning commented 10 years ago

Hi Tim,

Thanks for removing the bootstrap-3 dependency. Would you mind pushing latest to atmosphere, please? The version up there still has it in there so its getting pulled in for me whenever I do an mrt update.

(Btw, we're still on bootstrap 2.)

mizzao commented 10 years ago

@alanning you can't use the latest version of bootbox with bootstrap 2, anyway. All 4.x versions only work with bootstrap 3.

alanning commented 10 years ago

Thanks Andrew, I've pinned it to v3.2.2 in my app.

mizzao commented 10 years ago

@alanning I think @TimHeckel pushed 4.2.1 to Atmosphere, but it's slightly misleading as it's just a commit from the master branch rather than any particular release. I think we should generally just follow the release schedule in the future so that version numbers correspond.

albertmatyi commented 10 years ago

Hey. Do you know why atmosphere still reports version 4.2.0? And there is no way to update via mrt. You can only use the release if you specify the specific branch/commit

http://atmospherejs.com/package/bootboxjs - still reports version 4.2.0 with bootstrap-3 as dependency

mizzao commented 10 years ago

It was probably tagged but not released, due to Atmosphere being down when Tim tried to do it. I can fix this shortly - I'm probably going to remove the 4.2.1 tag as well.

mizzao commented 10 years ago

For your convenience, I've tagged and published the version that Tim released as 4.2.1-master.1 since it was actually pulled from the bootboxjs git repo and not a versioned release. Since Meteorite is using semver now, it'll count as lower than the next possible release version which would be 4.2.1 (or 4.3.0).