Skip to content
This repository has been archived by the owner on Sep 11, 2023. It is now read-only.

Bagupdate cleanup #56

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

syndenbock
Copy link

This merge cleans up the bag update handling to be more streamlined and easier to understand and also applies the the collecting of BAG_UPDATE events to the bag class.

@Jaliborc
Copy link
Owner

Without a detailed description, it's hard to understand what you're trying to acomplish. The use of the delayed event queue is cool, just never bothered to deal with it since the event was introduced way after bagnon was coded.

However, there are a lot of obvious issues with these changes. Some I found:

  • Cache events aren't being used, so API may not be ready before update.
  • Each instance of the Bag class ends up having to handle the event and an internal queue itself, which is not its job to do
  • The concern over what really needs updating is dropped by calling content and size events always and simultaneously, causing the double of the updates required

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants