Score calculation bug

[martindholmes]

What is currently SSResultSet.js line 247 is this:

return total + b.weight;

and it should be this:

return total + parseInt(b.weight);

This can cause scores of 1111111 (concats instead of sums) in certain edge-cases.

[martindholmes]

We might consider back-porting this fix to 1.4.

[joeytakeda]

Ah interesting! Good catch--yes, I think we should as a hotfix

[martindholmes]

I think this only gets triggered when something else is slightly broken -- I had inadvertently deleted the wrapper div for my search-only-in controls, which resulted in an undefined for their identifier -- but we don't want it lurking.

Now, how do we do a hotfix with our current main, dev, and active feature branch scenario? Will it work to fix in dev, then cherry-pick into the other two?

[joeytakeda]

You could do the fix in a branch off main, merge it into main, release 1.4.1, then merge main back into dev, and then merge dev back into the feature branch?

[martindholmes]

That's the way to do it. It'll give me the opportunity to test the release documentation too. Will do that this morning.

[martindholmes]

Release done, but that was not the right way to do it; because I had to update the ODD/documentation in dev and main, I now have a merge conflict between dev and iss218-deletions. This convinces me that we should follow the principle we have in the past whereby in feature branches we only work on code, and we following a merge into dev, we update the documentation only in dev.

[martindholmes]

I have resolved the conflicts in the iss218-deletions branch, but we may have more issues when we try to merge that back into dev. I think the iss218-deletions branch has too much stuff going on in it, and we should get it merged asap and confine our branches to single-issue items in future. Are we done with the initial deletion work at this point?

[joeytakeda]

Thanks for doing this! It's not clear though how this was done: I'm not sure why you had to change the EDITION file? I.e. I thought it would be something like:

git checkout main
git checkout -b hotfix
// Fix the single line in the JS and update the documentation
git add; git commit; git push
git checkout main
git merge hotfix
git push
git checkout dev
git merge main
git push

In any case, I'm not sure that making documentation changes only in dev will resolve any future issues—wouldn't the conflict will just happen between main and dev, not dev and the feature branch?

Though I agree that we shouldn't commit the built documentation—I had presumed that since it isn't in the .gitignore (but the other docs are), then we should be committing it, but that's not right. I'd say we should add it to the .gitignore and force push the HTML documentation (if we want it in the repo at all) as part of the release process.

[martindholmes]

I had to change the EDITION file to 1.4.1 for the release. If I didn't merge that into dev then re-change it afterwards, we would have had conflicts merging dev into main in the future because the preceding change to 2.0.0alpha in dev would have preceded the change to 1.4.1 in main. I also had to update the documentation to show the change in 1.4.1, so ODD, RNG and HTML got changed in dev/main; the problem was merging those changes into the iss-218 branch because there were already lots of additions to the documentation there.

Remember that the schema is also built from the ODD and that has to be in the repo; and also that the HTML documentation is the user-level documentation that travels with the repo checkout, so I think they both need to be tracked; we can't assume that users will build the ODD file after cloning just to be able to read the documentation. So I don't think we can gitignore them.

Our process is really based on the assumption that once we do a release, we won't do another one until all feature branches are merged back into dev, and it will be a clean merge from dev to main. But once we have documentation changes in dev or in feature branches that should not be included in a main hotfix release, we have a bit of an issue.

[joeytakeda]

Re: EDITION changes--that makes sense (I suppose one could do something fancy with rebasing and replaying an old commit atop a new one, but that's always seemed like dark Git magic to me).

Since this bug has been fixed, I'm going to close this ticket (though, re-open if you think it needs to be) and open up a new ticket to discuss the generated products / documentation / release protocols—we should get those pinned down and explore options for trying to mitigate these kinds of conflicts (while I'm hoping that we have very few hotfixes that would be necessary, it seems inevitable that this kind of thing will crop up again).