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
Implement Transactional flag #789
Conversation
74bb276
to
a262fcb
Compare
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.
So far, LGTM. But have a look to these console.log
. I think they are no really needed :)
For checking that 'with snapshots' and 'read-only' legends are properly rendered according to the Btrfs volume configuration.
@jreidinger I have added few unit tests for the UI part. Hope you don't mind. Please, have a look. |
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.
LGTM
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.
Sorry, I think this one deserves an entry in the changelog files
I have concerns regarding the wording. This is not exactly a "read-only" filesystem. I will elaborate (entering a meeting now) |
@ancorgs OK, I will wait. |
To be precise, it means the root subvolume of the Btrfs file-system is read-only. But that does not apply to the whole file-system. Other subvolumes on the same Btrfs file-system (like /home, /opt, /srv or /var) are still writable. If I see a Volume with an attribute I don't know how to name the D-Bus property or the label in the UI, but I don't like the current ReadOnly and read-only. Maybe we should try with "transactional". |
@ancorgs OK, makes sense. So lets use transactional name instead. I am just not sure if it btrfs exclusive |
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.
LGTM
@dgdavid Can you please also recheck? as it looks like your requests to change blocks merge |
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.
LGTM, just check the comment for translators
Co-authored-by: David Díaz <1691872+dgdavid@users.noreply.github.com>
Problem
There is no indication if btrfs volume will be read only or not.
Solution
Implement it.
Testing
Screenshots
Note: No suitable icon in materials not found ( at least I do not like any of them ).