Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Library: Add last_played_at column #3140
Library: Add last_played_at column #3140
Changes from 13 commits
3df10ad
ef647fc
ab83cd1
f936a34
43e5e63
3ecff63
98cc65f
ab7f187
2ae67da
cc48052
7c3b16c
4f9d766
66ea808
46a9857
cd86537
4fc531e
0625288
a01c978
5e40a18
8baa952
36b2dd5
b4b96fe
582b43f
59371b6
4ee3e85
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please don't invent a new log level. "debug" is our verbose level, just use that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this to prevent log spam. DEBUG is currently used as INFO. Explained here #2782. Otherwise I would have to delete all those extra logs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that we don't have a good story for Info vs Debug. But this feels like a one-off change that will increase tech debt. If you are going to do this, please create a bug and assign it to yourself to remove this special casing once we have better logging code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need also consider the played flag.
Maybe we can dispose it internally by comparing the current play count with the value the session starts. That sounds less redundant.
I did not have a look, but we have also a feature to join two set logs, the played flag needs to be considered than as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The played flag should not be a property of the (persistent) track object. The set of tracks that are considered as (manually) played during a session should be managed separately. The complicated and inconsistent logic in the
PlayCounter
is already screaming out loud. I refuse to add more magic to it that fits some use cases but induces unexpected behavior for others. If the user decides to rewrite history they could be bothered with updating the played flag manually. It will be reset anyway at some time.For my aoide view I don't have a played flag and already use the logic you proposed for an implicit played flag. This could be overridden with a session played flag, once Mixxx supports this concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the played flag is a slightly different idea and should remain separate. (Also note that Played status is maintained across crashes so the user doesn't lose state :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mangling the played flag as a column of the Track entity table was a bad idea. Instead the set of played tracks must be stored separately by introducing the concept of a Session entity. Out of scope of this PR.