Skip to content

added read_only attribute to base StorageStyle class - #3231

Closed
slogsdon7 wants to merge 4 commits into
beetbox:masterfrom
slogsdon7:master
Closed

added read_only attribute to base StorageStyle class#3231
slogsdon7 wants to merge 4 commits into
beetbox:masterfrom
slogsdon7:master

Conversation

@slogsdon7

Copy link
Copy Markdown

Worked on as part of addressing #3216 in a backwards-compatible way.

The way I did this was to modify the base StorageClass to take a named argument "read_only." If the argument evaluates too true, then the store() method on the instance is set to a lambda which simply returns None.

Example usage: MP4StorageStyle('----:com.apple.iTunes:Label', read_only=True).

For the moment, read only styles should be placed after the other styles of the same type in the argument list, as MediaField's get method returns on the first tag style it reads from the file.

@arcresu arcresu added the mediafile Relates to MediaFile and should be migrated to beetbox/mediafile label Apr 29, 2019
@arcresu

arcresu commented May 19, 2019

Copy link
Copy Markdown
Member

This change looks good to me! The only issue is that one of your lines is too long and if failing the style check. Would you mind fixing that up? It might also be nice to add a test for this, but I won't insist :)

@slogsdon7

Copy link
Copy Markdown
Author

This change looks good to me! The only issue is that one of your lines is too long and if failing the style check. Would you mind fixing that up? It might also be nice to add a test for this, but I won't insist :)

Done and done :). Gotta head out before these CI checks finish but if they catch anything else I'll fix them later today

@arcresu

arcresu commented May 31, 2019

Copy link
Copy Markdown
Member

The MediaFile split has happened now, so I'll close this in favour of beetbox/mediafile#9. Thanks for handling the migration!

@arcresu arcresu closed this May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mediafile Relates to MediaFile and should be migrated to beetbox/mediafile

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants