Skip to content

Immediate version info update on CBLoader#339

Merged
davidlatwe merged 5 commits into
getavalon:masterfrom
davidlatwe:cbloader
Sep 19, 2018
Merged

Immediate version info update on CBLoader#339
davidlatwe merged 5 commits into
getavalon:masterfrom
davidlatwe:cbloader

Conversation

@davidlatwe
Copy link
Copy Markdown
Collaborator

Before:

cbloader_before

The change of version in subset view, require re-select subset so the version widget will update.

After:

cbloader_after

Now the version widget will update immediately with reasonable database query counts.
You can scroll the version list and view comments smoothly.

Minor tweak

  • Supporting unicode comments.
  • Version widget will expanded by default.

@mottosso
Copy link
Copy Markdown
Contributor

Nice one David. I've added write permissions to you, so you can assign a label and request a review from e.g. @BigRoy. You're able to merge as well, just bear in mind to let everyone get a chance to see a PR first, at least 24 hours since the last commit.

@davidlatwe
Copy link
Copy Markdown
Collaborator Author

Big thanks @mottosso !
I'll keep that in mind :D

@davidlatwe davidlatwe requested a review from BigRoy September 18, 2018 06:15
@BigRoy
Copy link
Copy Markdown
Collaborator

BigRoy commented Sep 18, 2018

David, this is sweet. Thanks!

I actually am working on another "version widget" that shows all comments, bu this is already great. Nice job.


self.setEnabled(True)

print("Querying..")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this for debugging purposes, or is this a logical message to display for the end user?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not for the end user, my motivation was to know when will the actual database querying happen after this implementation, I thought it would be nice to know that for developer, so yeah, it should count as debugging purposes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - we can keep it. If it's too verbose later on we can always tone it down.

@BigRoy
Copy link
Copy Markdown
Collaborator

BigRoy commented Sep 18, 2018

Looks good to me, feel free to merge.

@davidlatwe
Copy link
Copy Markdown
Collaborator Author

Merging this :)
Thanks @mottosso @BigRoy !

@davidlatwe davidlatwe merged commit 5c8832f into getavalon:master Sep 19, 2018
@davidlatwe davidlatwe deleted the cbloader branch September 19, 2018 12:24
tokejepsen pushed a commit to tokejepsen/core that referenced this pull request Jul 30, 2021
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

Successfully merging this pull request may close these issues.

3 participants