Remove minimize option #741
Conversation
|
Do we need something else for this PR? Or you just don’t want to release at the night? |
Also i remove some other options, I think tomorrow we will do release. I hope I will not be hated for this. |
|
For removing features we need very good documentation around why this has to be done. Is this comment (#739 (comment)) the reason why this is being removed? If so, this PR's description should include that statement and also provide direction for users who look at this PR, for how to move to another loader for that feature. |
|
@shellscape you mean add love to ChangeLog? |
|
@ai changelog updates for breaking changes is required, yes. but PRs should have good info in them as well. I had to do some digging to find out why this PR was necessary, even with the link the PR description - users should not have to dig. Being verbose is good. |
There was a problem hiding this comment.
The code changes look good, and the feature looks like it is cleanly removed. But in order to approve this PR I would need to see that the README has been updated to explain that the feature has been removed (just removing a section is not good for user experience) and what the migration path is for users who relied on that feature. Adding that information to the CHANGELOG or Github Release would also be helpful. But users are going to turn to the README first, as they always do.
|
Can I ask you to write docs? I am not native speaker. |
|
@shellscape what section in docs you recommend to describe changes? Seems like all previous changes was not described in docs. And we should just use ChangeLog. @evilebottnawi can we accept it and write reasons in ChangeLog? |
|
@evilebottnawi are you plan to release 1.0 today? |
|
@ai yes |
|
@evilebottnawi sure. I think it is better plan. |
What kind of change does this PR introduce?
feature removing
Did you add tests for your changes?
tests were removed
If relevant, did you update the README?
docs were removed
Summary
@evilebottnawi suggested just to remove
minimizeoption to fix problem with old Browserslist #739Does this PR introduce a breaking change?
yes