Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3.11.2 - outstanding pieces #1773

Closed
13 tasks done
justinclift opened this issue Mar 1, 2019 · 67 comments
Closed
13 tasks done

3.11.2 - outstanding pieces #1773

justinclift opened this issue Mar 1, 2019 · 67 comments
Milestone

Comments

@justinclift
Copy link
Member

justinclift commented Mar 1, 2019

@justinclift justinclift added this to the 3.11.2 milestone Mar 1, 2019
@justinclift justinclift added the release blocker Bugs that are serious enough to block our next release label Mar 1, 2019
@justinclift justinclift pinned this issue Mar 1, 2019
@MKleusberg
Copy link
Member

I consider #1733 (and related to it e52f205) to be a serious problem too. ... which reminds me to cherry-pick e52f205 to the 3.11.x branch 😉

@justinclift
Copy link
Member Author

Thanks, just added those. 😄

@justinclift
Copy link
Member Author

justinclift commented Mar 1, 2019

Hang on... #1733 looks like it was merged in time for 3.11.1. Yep, it's in the 3.11.1 release notes, so just removed it from our list in this one. 😄

@MKleusberg
Copy link
Member

Oh you're right! 😄 But there's still e52f205 which breaks the grammar parser in similar ways.

@justinclift
Copy link
Member Author

Yeah, that's why I added 1a59c8c. That's the 3.11.x version of that commit yeah?

@MKleusberg
Copy link
Member

Yep, it is 😄

@justinclift
Copy link
Member Author

Thanks. 😄

@mgrojo
Copy link
Member

mgrojo commented Mar 2, 2019

#1772 should be fixed in 0adb0af and I think it could be brought to the release branch.

@MKleusberg maybe you could take a look to the solution, just in case you come up with a better approach or still pending gaps. I was surprised by the fact that QString QTextCodec::toUnicode(const char * input, int size, ConverterState * state = 0) const ends at the first zero in input instead of continuing up to size. Do you think it could be a bug in Qt?

@mgrojo
Copy link
Member

mgrojo commented Mar 2, 2019

@mgrojo The QDarkStyle fix looks like it might work ok... not sure if the code is suitable for our stable/release branch or not though?

It might be ready for release, but it contains a lot of changes. Maybe the cherry-picking raises conflicts.

@justinclift
Copy link
Member Author

Maybe the cherry-picking raises conflicts.

Yeah, I haven't checked yet. Was more thinking that even if it doesn't raise conflicts... not sure that we've really had many people testing it yet. Kind of nervous to put it in, this far along the stabilising of a major release...

... on the other hand, a lot of people do seem to want a dark theme available.

Thoughts? 😄

@MKleusberg
Copy link
Member

@mgrojo I think your commits looks reasonable. I have no better idea either 😄 But I don't think it's a bug in Qt. In the docs they say

Converts the first size characters from the input from the encoding of this codec to Unicode

which to me sounds like the size parameter doesn't determine the length of the input but only how much of the input should be converted. So while I think what we want would make more sense, this doesn't sound like a bug in Qt.

@mgrojo
Copy link
Member

mgrojo commented Mar 3, 2019

@MKleusberg I guess they'd claim is working as expecte, but it is a bit inconsistent, because some codecs will stop at the first zero and others not, because that is a valid encoding pattern, like in UTF-16. In fact, I've checked that the same behaviour is in the ISO-8859-1 and I guess in all of the ISO-8859 encodings.

    if((encoding.isEmpty() || encoding.startsWith("ISO-8859")) && data.contains('\0'))
       return false;

Probably a lot more encodings don't use embedded zeros, but I think we can leave it as above, unless someone comes up with a better idea.

mgrojo added a commit that referenced this issue Mar 3, 2019
@justinclift
Copy link
Member Author

justinclift commented Mar 6, 2019

It looks like #1772 (comment) still has some fine tuning to do.

I'll move the release date for 3.11.2 back a week, so there's some breathing space to investigate, etc.

I'm not personally ready to make the release yet anyway, as I'm focusing on getting a first working version of dio released. In the final stretch for that (maybe today or tomorrow?) so ignoring most of everything until then. 😄

@justinclift
Copy link
Member Author

@MKleusberg The patch for #1758 isn't in the v3.11.x branch yet. Want to cherry-pick it across? I can do it if you can't be bothered. 😄

@MKleusberg
Copy link
Member

@justinclift No worries, I have just cherry-picked it 😄 Thanks for pointing this one out by the way!

@mgrojo I completely agree with everything you said 😄 At some point it probably makes sense to change the entire binary detection code anyway, so for now this is hopefully good enough.

@justinclift
Copy link
Member Author

In theory, we should be releasing 3.11.2 tomorrow or so.

  • How do people feel about it's current state?
  • Are there other issues/PRs/etc that should be included?

I'm not sure on the status for #1772. It kind of sounds like the patch there introduced incorrect character counts, and I'm not sure what's going on with that. 😉

mgrojo added a commit that referenced this issue Mar 12, 2019
@mgrojo
Copy link
Member

mgrojo commented Mar 12, 2019

I'm not sure on the status for #1772. It kind of sounds like the patch there introduced incorrect character counts, and I'm not sure what's going on with that.

The incorrect character counts (in fact, stripping of CR characters), predates #1772 and it's even present in v3.11.x. The solution (#1793) is too disruptive to be added, because it involves replacing the Qt widget by the QScintilla one.

I've added the two commits fixing #1772 to the v3.11.x branch.

@justinclift
Copy link
Member Author

Thanks @mgrojo. Sounds like we've done the best we can with that for the v3.11.x branch then, so that'll have to do. 😄

I'll build our macOS & Windows release binaries for 3.11.2 today or tonight, so we're good to release when @MKleusberg has time to sign the Windows bits. 😄

My focus stuff (the dio command line utility) is taking a lot longer than I was hoping for. The first pass of it was finished last night, but it really needs unit tests added to catch at least the straight forward bugs. And there will be bugs. 😉

@MKleusberg
Copy link
Member

@justinclift Did you have the time to build the binaries yet? I could easily sign them this week. But any later day would work too 😄

@justinclift
Copy link
Member Author

Did you have the time to build the binaries yet?

Not yet. Super focused on getting the the unit tests for dio finished (should be done tonight) so it's releasable. There are ~30 pieces of dio to write tests for, and I'm on the last one now (as of last night).

I'm thinking the binary building will probably be friday. 😄

@justinclift
Copy link
Member Author

Binary building is going to be tomorrow instead of today. I've just now finished the dio bits I wanted to get done, but I'm pretty sleepy and that's not the right time to be building a software release. 😉

@justinclift
Copy link
Member Author

It sounds like we still have problems with attaching databases (#1799), so one of the main issues for 3.11.2 (#1758) isn't completely fixed yet. 😱

@beRto-
Copy link

beRto- commented Mar 25, 2019

@justinclift - @mgrojo completed a fix for #1709 on Feb 23. Can you please advise if that is getting picked up on the 3.11.2 release? I don't see it in the list above.

@justinclift
Copy link
Member Author

I've no objections to including it, if @mgrojo feels it's a decent idea. 😄

@justinclift
Copy link
Member Author

Just in case it helps, I've regenerated the .ts files in the v3.11.x branch. It still has the "unfinished" strings, so should I apply your PR to this as well?

  https://github.com/sqlitebrowser/sqlitebrowser/blob/v3.11.x/src/translations/sqlb_it.ts

@GortiZ6
Copy link
Contributor

GortiZ6 commented Apr 1, 2019

Ouch! You're right, I've done the changes from the "master" branch. I haven't checked the v3.11x branch, anyway if the strings are a subset of master, you can pick them from there :) Thanks.

@justinclift
Copy link
Member Author

Thanks @GortiZ6. 😄

I'll get that done later one today, before creating the new 3.11.2 release builds. 😄

@justinclift
Copy link
Member Author

Done in d389a62. 😄

@justinclift
Copy link
Member Author

Started getting this done, but need sleep. Will pick this up in the morning instead.

@MKleusberg
Copy link
Member

Ok, give me a minute 😃

@MKleusberg
Copy link
Member

And it's done 😄 They are all signed and added to the release draft.

@justinclift
Copy link
Member Author

Excellent, thanks. I'm still updating bits on the mac build server... it's nearly done. 😄

@MKleusberg
Copy link
Member

Awesome 😄 So we might get a new release today if things go well 🍾

@justinclift
Copy link
Member Author

justinclift commented Apr 3, 2019

Yep, just published it: https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.2

I'll get the README + website updated (etc) over the next few hours. 😄

@justinclift
Copy link
Member Author

justinclift commented Apr 3, 2019

Now unpublished. Guess who forgot to update the version string in our source before generating binaries... yep me. Dammit.

Ok, getting that done now. *sigh*

@justinclift
Copy link
Member Author

Just added the (fixed) macOS .dmg to the draft release. Feel free to hit the publish button when the new windows binaries are signed (etc). 😄

@mvt91
Copy link
Contributor

mvt91 commented Apr 3, 2019 via email

@justinclift
Copy link
Member Author

@mvt91 Ahhh, we've already created the release binaries, so maybe next time instead? 😉

@MKleusberg
Copy link
Member

Heh heh, no problem! I have just signed the new binaries and will publish the updated release in a second.

@justinclift
Copy link
Member Author

Thanks. I'll get the rest of the bits done (eg download cluster, website post, etc). 😄

@justinclift
Copy link
Member Author

@deepsidhu1313 @lbartoletti We've just released DB4S 3.11.2, if you're interested in updating your packaging bits. 😄

    https://sqlitebrowser.org/blog/version-3-11-2-released/

@justinclift
Copy link
Member Author

Hmmm, just realised we should be including an AppImage with this release.

Um... @deepsidhu1313, any interest in building it and adding it directly to the 3.11.2 GitHub Release?

    https://github.com/sqlitebrowser/sqlitebrowser/releases/tag/v3.11.2

@deepsidhu1313
Copy link
Contributor

deepsidhu1313 commented Apr 3, 2019 via email

@justinclift
Copy link
Member Author

Thanks @deepsidhu1313. When you've added it to the release info, please let me know. I'll then copy it to our download cluster for that to be the main download source for it, so we can count downloads ourselves instead of having to rely on stats from GitHub. 😄

@justinclift
Copy link
Member Author

I think everything I needed to do for the release is done now, and all of the appropriate people notified. Time to get some sleep. 😄

@mvt91
Copy link
Contributor

mvt91 commented Apr 4, 2019 via email

@deepsidhu1313
Copy link
Contributor

deepsidhu1313 commented Apr 4, 2019 via email

@justinclift
Copy link
Member Author

No worries @deepsidhu1313, Real Life takes priority. 😄

@justinclift
Copy link
Member Author

Closing this issue, as 3.11.2 is out and nothing seems to have broken (badly). We can keep updating this with info if needed, and/or open new issues where appropriate. 😄

@justinclift justinclift unpinned this issue Apr 4, 2019
@justinclift justinclift removed the release blocker Bugs that are serious enough to block our next release label Apr 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants