Dogfalo / materialize

Materialize, a CSS Framework based on Material Design
https://materializecss.com
MIT License
38.87k stars 4.74k forks source link

Refactoring public methods should be managed as major changes #3865

Closed GuillermoBlanco closed 7 years ago

GuillermoBlanco commented 7 years ago

The refactored modal plugin method breaks backward compatibility https://github.com/Dogfalo/materialize/commit/4984e2391fa7ce428d77910c2656835c021d11be#diff-452a8aa82569876ba89deba84331011cL183

 $('.modal').leanModal();  -->  $('.modal').modal();

So the release version should be incremented as the npm docs recommend https://docs.npmjs.com/getting-started/semantic-versioning

ilan-schemoul commented 7 years ago

(!!) Indeed that explains my modals wasn't working anymore since weeks. I think it's a shame really to make such changes and pack it into a minor version. I've try to searched : open modal is not a function materialize and didn't found anything because I and other devs cannot guess that you change public functions and only upgrade a minor version. Thanks @GuillermoBlanco I would have never guess why app wasn't working well, or not before a long time, without your message. What a shame to be such unpredictable.

levino commented 7 years ago

This is outrageous. v0.97.8 should be unpublished immediatly and released as v0.98.0. Thank you for wasting my time finding this!

ilan-schemoul commented 7 years ago

How long do the team will ignore such an issue. The problem is not to unpublish/republish but to be careful when you change a public method on an API used by thousands devs including in production

kmmbvnr commented 7 years ago

@NitroBAY and @Levino can you be a bit respectful of the hard work that maintainers do?

Everyone makes mistakes, and actually, this one was no so very hard to find and fix for you.

Maintaining an open source project is a source of constant abuse from ungrateful users. Some toxic users start thinking that you're under obligation to give your time for free and that's very disappointing.

I'm leaving here two links, I hope both of you can read them and better understand the consequences.

https://www.reddit.com/r/programming/comments/5c3f7q/brett_cannon_why_i_took_october_off_from_oss/ http://www.snarky.ca/why-i-took-october-off-from-oss-volunteering

levino commented 7 years ago

I know what open source is. But a project with 1000 open issues and people releasing braking changes as patches should be discontinued, if people can not keep up with the popularity. This very big issue has not been fixed for more than a week. Also where are tests that prevent this kind of regression?

Open source does not mean “not to be used in production“. One can expect people not to break stuff.

ilan-schemoul commented 7 years ago

@kmmbvnr you may not be a native speaker because you have a pretty weird definition of ungrateful, I'm grateful and I show it (three PR more than you for instance).

[...] constant abuse from ungrateful users. Some toxic users [...]

Tell me what is ungrateful by saying that Materialize team should be more careful ? I said :

How long do the team will ignore such an issue. The problem is not to unpublish/republish but to be careful when you change a public method on an API used by thousands devs including in production

You said that I've been thinking that maintainers are :

under obligation to give your time for free

If a lot of website depends on it and you didn't drop support nor pass the maintainer role to another guy you have to not break these numerous projects that depend on it. In which way am I asking to spend time, I'm not even telling them to rebuild (which could take .. oh my god ... few minutes). http://www.dictionary.com/browse/ungrateful?

To resume I'm grateful but when a big project cannot break the API without taking some prevention (actually just change major version, not minor one). Don't mix me up with Levino, I don't agree with his way of telling what we're all agree on. Also see #3825 to see if I'm a toxic user and next time before being agressive for no reason think a little bit.

kmmbvnr commented 7 years ago

a project with 1000 open issues and people releasing braking changes as patches should be discontinued

That's actually a good example of toxic behavior.

As a maintainer of several Python packages, I just can say that the first @GuillermoBlanco and several :+1: under it was completely enough to raise an issue. If anyone other thought that writing things like a shame, thank you for wasting my time, a project should be discontinued and :+1: under such kind of messages will help somehow to maintainers to resolve this, it's not true.

I'm not a javascript guy, but I've got the same problem several times already with other packages. Ex, this one was related a minor update in Babel. I don't think that Babel project author is wrong in some way, even b/c the issue was never resolved.

The yarn.lock was introduced exactly b/c that's the default npm behavior is the problem for everyone.

Dmitrev commented 7 years ago

@Levino @NitroBAY If your company depends highly on Materialize, it makes sense to invest in it financially.

Dmitrev commented 7 years ago

A workaround could be to just make the missing functions yourself like this, this may vary per code base: https://jsbin.com/hatujuvewo/1/edit?html,css,js,console,output

It's kind of trail and error at first to get it working. It makes sense to create your own wrappers for Materialize, which then interact with materialize. I got my own application back to normal almost, which is quite large.

EDIT: text and updated link with seperated JS

yanickrochon commented 7 years ago

The release of the revision (0.97.7 > 0.97.8) that introduced breaking changes is indeed a mistake. All projects should follow semver specs and, in this case, the changes should've kept the old API as aliases and simply marked it as deprecated.

Such mistakes is not the first to happen, and will certainly not be the last. The difference between open source projects and commercial ones is that, at least, open source projects allow people to contribute; you're not happy with something, contribute! Whining about it just marks you as an entitled jerk, and contribute to the wrong perception of people that open source is bad. (Because it makes sense that some people believe that PHP is better than Python... while arguing that open source is less trust-worthy.)

As a developer, it is my role to convince my boss that, by contributing to open source, I cut project cost and time considerably. And this project requires more code reviews! Some PR should be merged or rejected, but not remain in the open for months!

ilan-schemoul commented 7 years ago

I'm not in a part of a entreprise, not a professional, I dont whine, I didn't had to make a lot of changes, I just really think thay they should change the major version when they change API. They decided to change minor version, it's a wrong decision as peoples lost time because of that and as it's a NPM package it's even more likely to break compatibility in a production app. I do respect open source and what this project is, I just share my mind on a decision.

Dmitrev commented 7 years ago

@yanickrochon If you know how to convince my boss to contribute to this project, instead of "production of features". I would like to hear :)

yanickrochon commented 7 years ago

@Dmitrev I do not know your boss, so I am not in a good position to counsel you on this. However, the way I usually work with my boss is this :

Me: I have two choices; 1) implement all the UI myself, with events, widgets, CSS, etc. etc. or 2) use an existing project, plug it into the app and start using it. Boss: Which of these is faster? Me: The second, of course! But this also means that, instead of implementing my own stuff, I need to spend some time here and there to add features that we need -- which also be beneficial to others, just as others would so as well for us -- , the collaborative efforts mean that I am not alone creating features, and many of which we can use 100% freely. Boss: Would these "new features" take time? What guarantee is there that others' features work for us? Me: There is always a minimal risk that adaptation is required, however the fact that I am not alone reviewing, testing, and implementing cuts costs in the long term. Boss: What long term? Me: A year, two years, or so. Since we invest in an app, now, we want this investment to be worth while, and having others maintain parts of our app is a good thing. Boss: I see.

Or something like that :stuck_out_tongue: Make your own arguments! lol

christopher-francisco commented 7 years ago

Is somebody maintaining this repository? I see the public API was broken on a release, 1k + issue opened. We're on the verge of a major design refactor, I'd like to know whether I should suggest to change front end framework?

kmmbvnr commented 7 years ago

Actually, if you take closer, most of the issues are questions from users and feature requests.

Looks like maintaitainers have a very gentle strategy and not close them aggressively.

acburst commented 7 years ago

It was an unfortunate oversight. We are still actively maintaining it, but our time is spread thin at the moment. In the near future, my time will be more free and I will put more effort into Materialize. I apologize for the error in versioning and I'll be more careful next time.

christopher-francisco commented 7 years ago

I see this is marked closed, but I don't get the resolution. Should we refactor our code and adapt to the new version? or was the new version fixed and is now backward-compatible? Is there any breaking changes list that we can refer to when updating this PATCH version?

tomscholz commented 7 years ago

@chris-fa No, we weren't planning on making this change backward compatible. You have to refactor :/ No, there is not. EDIT: From what version are you coming?