2.2 release discussion #479

Closed
mbest opened this Issue May 11, 2012 · 40 comments

Projects

None yet

5 participants

@mbest
Member
mbest commented May 11, 2012

I'd like to get the ball rolling on version 2.2. I just finished going through all the open issues. 36 of them I closed, mostly invalid, already fixed, or duplicates. 48 of them I have marked for inclusion in 2.2. Just like for 2.1, I've also divided them based on "impact" and whether they're "bugs" or "enhancements". In addition, I marked some issues for version 2.3+ and a few as "future".

I also set up a 2.2 milestone to track our progress.

@mbest
Member
mbest commented May 11, 2012

I'd really like to keep the issue list clean by re-using the existing issues for the pull requests. Github doesn't have a way to do this though the web UI, but it's really easy to do through the API.

Steps:

  1. Create a new branch and commit code to it (include issue number in branch name to make it easy to track).

  2. Push branch to Github.

  3. Use curl to connect branch to issue (need branch name, issue number, and your username/password):

    curl -v -X POST -H "Content-type: application/json" \
        -d '{"issue":"$issue","head":"SteveSanderson:$branch","base":"master"}' \
        -u "$username:$password" \
        https://api.github.com/repos/SteveSanderson/knockout/pulls
@mbest
Member
mbest commented May 12, 2012

I've uploaded code for #138, #115, and #326. These are ready to merge.

@rniemeyer
Member

Nice work Michael!

@mbest
Member
mbest commented May 12, 2012

Nice work Michael!

Thanks. @rniemeyer You might be interested in what I did for #115.

@rniemeyer
Member

Yes, I looked through all of the changes last night. I think that both #115 and #326 will be quite helpful.

@mbest
Member
mbest commented May 13, 2012

@SteveSanderson - please review/merge the above issues. I'd like work on other ones but would like to avoid ones that have to be manually merged.

@mbest
Member
mbest commented May 14, 2012

@SteveSanderson - Three more are ready to merge: #459, #460 and #144.

@SteveSanderson
Contributor

Nice one. I'll look through these tomorrow. Thanks for taking the
initiative on this!

On 14 May 2012, at 21:52, Michael Best
reply@reply.github.com
wrote:

@SteveSanderson - Two more are ready to merge: #460 and #144.


Reply to this email directly or view it on GitHub:
#479 (comment)

@mbest
Member
mbest commented May 16, 2012

@SteveSanderson - Thanks for merging #460. Still waiting for #138, #115, #326, #459, and #144. Especially #326 and #144 are important since I'm waiting on them before working on some other issues.

@wachunga

Excited about #115.

@SteveSanderson
Contributor

I've just done a triage pass through all the issues labelled "2.2" to make sure that, for each one, I've either agreed with the proposal, am waiting on a question, or in the most impactful cases where I can't quickly make an assessment I've labelled steve-to-review.

For the ones I've commented "Agreed we do this" or similar, that's not a request for someone else to do it, it's just a note that I'm satisfied it passes the cost/benefit threshold for inclusion in 2.2. Soon, I'd like it if we could get into the habit of using GitHub's "assign to" feature to distribute out some of these work items to me, Michael, and anyone else who wants to write patches. We might want to plan monthly milestones or similar (working towards a 2.2 release in a few months), so that a certain batch of items can be processed in a known amount of time.

I'll also be posting some further thoughts about fixing some processes and engineering debt shortly.

@mbest
Member
mbest commented May 22, 2012

Thanks, Steve. I also think we should use "assign to" and plan monthly milestones.

@mbest
Member
mbest commented May 31, 2012

@SteveSanderson, Can you please make a priority of reviewing #326 and #144? I'm sitting on code for related issues, waiting for those two to be finalized.

@mbest
Member
mbest commented May 31, 2012

@SteveSanderson - I noticed when going through the issues that the labels for some of them were messed up (they had both bug and enhancement, or both minor and moderate impact). Most likely this happened when you added the "steve-to-review" label by checking multiple issues in the issue list and then setting the label. This method doesn't work for adding a single label to a bunch of issues. Instead it will set the labels for all checked issues to be the labels already set for any of them. So only set labels for multiple issues at once when you want the labels for all of them to be exactly the same.

I fixed the labels already for the ones that were messed up.

@mbest
Member
mbest commented Jun 6, 2012

I've added code for #352 and also added a new pull request: #512. Both are ready to merge.

@mbest
Member
mbest commented Jul 3, 2012

I've opened an issue (#552) to address the binding order problems in Knockout and scheduled it for after 2.2. I'd originally scheduled three separate issues for version 2.2 dealing with binding ordering, but I think this problem deserves a more thorough investigation and discussion that would be better left for the next release.

@mbest
Member
mbest commented Aug 1, 2012

We've got 37 issues still scheduled for version 2.2. Probably if we want to release this by the end of September or early October, we'll have to cut some of those out. In the last two and half month, just counting issues that we implemented/merged, we've completed only 25.

@mbest
Member
mbest commented Aug 1, 2012

I went through the list now and reduced the number to 31. Still might not get them all in.

@SteveSanderson
Contributor

It would be great for us to put out a 2.2 release really soon, preferably within a month or so, because it's been a long time since 2.1 and there are some great features in master right now that people are waiting for.

To make this possible, I suggest we just take the top few highest-urgency items that we can realistically manage during September. Items that don't make this cut are still valid, but we have to be pretty aggressive in selecting only a small number of issues otherwise it's just not going to happen.

I've just been through all items assigned to the 2.2 milestone and grouped them into "bugs", "enhancements", and "doc issues", and stack-ranked the items in each group by urgency. For "bugs" and "enhancements", I'm also suggesting a "cut line" that would define the new 2.2 milestone. Things below the line would be reassigned to 2.3. Michael and Ryan, what do you think?

Bugs (high priority first)

#393 - Memory leak with native foreach
#551 - select scroll position issues in IE (again)
#351 - Line 1363 : ko.selectExtensions.writeValue - IE 6
#324 - Problem setting domnode value in IE 9
#615 - Regression with jquery.tmpl
--- CUT LINE ---
#376 - Extend memoizeDataBindingAttributeSyntaxRegex to accomodate spaces around the equal signs
#462 - bindingHandler.value's valueHasChanged field is 'false' but maybe should be 'true'
#355 - hasfocus won't set focus if element is hidden
#91 - data-bound VML does not work within the template binding

Enhancements (high priority first)

#483 - binding system should only use computed when there are dependencies
#476 - make lightweight version of 'with'
#601 - use the new 'peek' or 'ignore' in a few more places; add peekObservable for bindings
#562 - Bug (?) in documentation on computed observables - writeable computeds don't have a chance to react to a change that is the same as current value
--- CUT LINE ---
#607 - compress code size (multiple methods)
#582 - Add ko.asyncComputed shorthand for a minimally-throttled computed
#251 - ko.toJS() serialize every property including ordinal functions
#485 - support observable view models at both the root and child context level
#484 - computed can be attached to multiple nodes and dispose when all are removed
#500 - Binding handlers should be able to use the init function to manage updates
#535 - update parseObjectLiteral for size and speed
#486 - fix google compiler warnings
#495 - Paying off engineering debt (this is really a long-term meta-issue describing multiple enhancements, some of which will happen sooner than others)

Docs (can handle all of these)

#374 - add documentation for "data-bind" attribute syntax
#372 - Tiny error in collections tutorial
#157 - update width on document template (not sure we want to do this though)

Let's not try to squeeze more items in, because the workload in updating docs and actually putting out the release is significant in its own right. Perhaps instead we can work towards making releases more frequent so each release workload is less of a big deal, and some of the cut items can get out in 2.3 sooner. Thoughts?

@rniemeyer
Member

Sounds like a fine plan. I think that everyone would agree that smaller, more frequent releases would be a good thing to do. I will certainly help out with anything that I have time for.

I would probably move #615 up. I know that it deals with jQuery Templates, but without a fix many people won't be able to upgrade. I have talked to many folks that are using jQuery Templates form earlier days and are not able to trivially move to native templates. I'll take a look at it that one.

Also, I feel that #562 is a bug and not a just doc bug. I can take that one too.

@mbest
Member
mbest commented Sep 3, 2012

I'd like to add #474. I hadn't originally included it in 2.2, but it's a small change that already has code, and it will help me with a plugin I'm working on that will implement #321 (also #601 will help so I'm glad you've included it). It also already includes a fix for #615.

I also agree with Ryan that #562 is a bug. Otherwise, the list looks good.

@flexiblefactory

Do you have any comment on pull request #524? This is a fix to a bug, and although the title of the pull request is slightly confusing, this bug is prone to occurring in real scenarios as this was how we discovered it. Please see the jsfiddle for more info.

@SteveSanderson
Contributor

Michael/Ryan: Please have a look at #661 - we'll need to include this (or something like it) in v2.2.0 to get the specs to pass on IE.

@mbest
Member
mbest commented Oct 9, 2012

@SteveSanderson: please take a look at #665. This change fixes a performance issue with foreach and makes the order of operations closer to that in version 2.1.0.

@mbest
Member
mbest commented Oct 9, 2012

Also I think we should include #660 in 2.2.0.

@mbest
Member
mbest commented Oct 9, 2012

I've created a branch, gh-pages-2.2.0 for documentation updates specific to 2.2.0.

I noticed that the gh-pages branch has a spec directory. Do we still need that?

@SteveSanderson
Contributor

OK I've merged in #660 and #665.

For #660 I had to make a further spec tweak because the new spec happens to trigger an entirely different weird behavior in IE<9 ("Unexpected call to method or property access") because of the particular shape of the malformed DOM we were testing. To fix this, I changed the malformed DOM to more closely simulate what Roy was reporting, and also had to change the dummy template engine to use ko.utils.parseHtml instead of its own parsing logic, otherwise the spec was no longer proving anything - it was passing even without fixing the defect. Overall, this was a mess, and I would like us to eventually put in the logic that throws errors and aborts if the DOM is malformed instead of trying to make malformed DOM shapes work in old IE.

I've run the specs again in all combinations of browsers and with/without jQuery and we are back to a 100% pass rate. Let's write the docs and ship this thing :)

@flexiblefactory

Michael/Steve, Have you looked at #524?

@mbest
Member
mbest commented Oct 10, 2012

Michael/Steve, Have you looked at #524?

I've looked at it but haven't made any decision on it. At this point we're not adding anything else for 2.2 except fixing regression issues.

@mbest
Member
mbest commented Oct 15, 2012

@SteveSanderson, #672 is a fix for a regression issue in 2.2.0rc. I think we should include this before release.

@SteveSanderson
Contributor

#672 is a fix for a regression issue in 2.2.0rc. I think we should include this before release.

Yep, OK, I agree. I didn't want to have to run the specs on every combination of browser again, but you're right that this would be an unacceptable regression that breaks common and basic scenarios, so I do think we should include the fix. Please merge it in if you're happy with the spec I added.

@SteveSanderson
Contributor

I've rechecked all the specs on all browser combinations - still all passing :) So we can release this as soon as the docs are ready!

@mbest
Member
mbest commented Oct 25, 2012

I'm working on the last part of the documentation changes and will have it ready in a couple days.

@mbest
Member
mbest commented Oct 27, 2012

I've uploaded the binding syntax page, which was the last documentation item. Please review it, and then we're ready to go.

@rniemeyer
Member

It looks good to me. It should be a useful page. Thanks for working on it.

@SteveSanderson
Contributor

I've uploaded the binding syntax page, which was the last documentation item

Generally looks great. I tweaked a bit of phrasing at the end to make it sound a tiny bit more upbeat - hope that's OK. The actual information conveyed is the same.

@SteveSanderson
Contributor

OK, fantastic, we're all set! I've just done the following:

  • Made the final build of knockout-2.2.0(.debug).js, committed it, tagged it, and uploaded it to the "Downloads" page on GitHub
  • Updated the gh-pages-2.2.0 branch to have the final build of KO and the final set of specs. Also merged the recent gh-pages changes into it, so the merge back will be trivial.
  • Updated the NuGet package to 2.2.0

What's left to make this "officially released" is:

  • Merge gh-pages-2.2.0 back into gh-pages (which will automatically update the public site, knockoutjs.com)
  • At the same time (or shortly after) send an announcement to the KO forum

I'm very happy to do these last two items, but I don't want to steal all the glory :) Michael/Ryan - would either of you care to push the big button to make it go live? Any time in the next day or so would be great.

When I see it go live I'll blog & tweet about it too (as I'm sure Ryan will too...).

@rniemeyer
Member

Doesn't matter to me. I say go for it Steve, if you would like.

@mbest
Member
mbest commented Oct 29, 2012

I tweaked a bit of phrasing at the end

Looks good.

I'm very happy to do these last two items

I agree with Ryan. Go for it.

@SteveSanderson
Contributor

OK thanks, done!

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