fix: Removed redundant "Exclude" and "Copy" menu options in LogViewDialog #1672
Conversation
|
Hello T1petitti , Looks good on the first view. I will test and review as soon as possible. @Sakh1l the related issue was assigned to you in December. Might it be that you have also worked on that problem? Will you add something to this PR or will you also review this PR? Best, |
| btnAddExclude = menu.addAction(_('Add to Exclude')) | ||
| btnAddExclude.triggered.connect(self.btnAddExcludeClicked) | ||
| btnAddExclude.setEnabled(cursor.hasSelection()) | ||
|
|
||
| btnDecode = menu.addAction(_('Decode')) | ||
| btnDecode.triggered.connect(self.btnDecodeClicked) | ||
| btnDecode.setEnabled(cursor.hasSelection()) | ||
| btnDecode.setVisible(self.config.snapshotsMode() == 'ssh_encfs') |
There was a problem hiding this comment.
It seems to me that only the entry "Decode" is left in this context menu. This is visible only when the current snapshot profile use encryption (EncFS).
So I propose to add the check self.config.snapshotsMode() == 'ssh_encfs' to line 147 where the context menu trigger is installed. It should be installed only when there is a encrypted (encfs) snapshot profile.
Side note: What confuses me is that the check is only for ssh_encfs and not also local_encfs. Maybe you can check this out if there is a good reason for this? But be aware that we plan to remove (replace) encfs in the long run (#1549). So don't invest to much resources into this encfs-thing.
There was a problem hiding this comment.
Hi. I worked with T1petitti in a group of 3. The Decode option does not come up graphically because the context menu does not appear either, which is why we didn't think we needed to change anything involving the Decode feature, but moving the condition to before the context menu is created instead of after is something we can do.
3bc25fa
This commit by @Germar is where it got added. It does not say why it got added.
Is it possible 'local_encfs' does not have a decode option because local file-paths, not through ssh, do not need to be decoded?
There was a problem hiding this comment.
Great! 🚀 A team of three. 🤟
I don't use EncFS and so I don't know the details. Might it be possible that you can give it a short test: Create one "locale (encrypted)" and "SSH (encrypted)" profile and then check this feature?
|
If we keep the "Decode" feature it would be nice it there is a short help text (a QLabel) in the dialog mention that; but of course only if it is an encfs profile. |
|
Because of the Decode feature I tested myself a "local encrypted" and "SSH encrypted" profile. The snapshot log of an "local encrypted" profile do show me the real path and file name. No need for decode. |
… keyword for the dbus method
…n alternative to the shutdown feature
|
Can you please give me a short feedback about the current status? Please don't feel any pressure I just need that info for our internal planing. |
buhtz
added
the
Feedback
…Shutdown,-Suspend,-Hibernate-after-taking-snapshot' # Conflicts: # common/backintime.py # common/tools.py
…BUS method in xfce
|
Pls take it |
buhtz
removed
the
Feedback
|
This PR is replaced by #1695 . I couldn't see further progress or valuable feedback. It surprised me a bit because you mentioned there is a team of 3 people working on this PR. Next time please open a PR only if you are willing to go the full length of the way. You don't have to walk alone in this project. The maintenance team will mentor you if you need. Please let me know how we can improve the handling of contributions and the communication. Best, |
…free disk space, dynamic
Master branch should have been main branch
…act_shutdown tooltip text
|
Dear Demetrios and T1petitti, Regarding your accidentally opened and later closed PR #1704 I am assuming that your commits in this PR here 11 hours ago are also an accident? Am I right? Or are they on purpose? Best, |
…or gnome to try and fail to suspend
…oolbar-items Give option to display text in toolbar items
|
It feels to me that @vozellad and @T1petitti do not read my comments. Let me know how we can improve our communication. Feel free to use email for example. We can switch to IRC as a live chat.
|
|
Apologies for the miscommunication. My group is part of a class project, and we misunderstood the project instructions. We didn't have to merge with the official project, so we moved on to other issues to work on in private like we were supposed to before. I apologize for not letting you know the nature of our work. |
|
Thanks for reporting back. My initial instructions also weren't not as precise as needed. ;) |
|
We only planned to push that first commit to here and have the other ones be private to only us and the professor. If we find a way to not have the commits show up here, we will do it. |
|
Dear Demetrios, I am interested in your class and the task your professor gave you. Maybe you can describe it a bit more? Did you asked your professor about this PR? To work in "private" you could have created a new branch after your first "public" commit. This second branch is not part of this PR so it won't be seen by us. Please tell me about your first commit. What exactly was the plan? I PR is not finished with just committing. Finetuning and discussions are part of a RP process in most cases. Did your professor told you that or realized it? Be aware that it distracts the maintainers (from there limited spare time) with throwing in just a bunch of commits and then not answering back. I hope you learned that. Best, |
|
Our project was to work on an open source project to get used to working on other people's code. There was no discussion for working with anyone else outside of the class. The class is called Senior Design & Development. Is it giving you notifications for the commits we're doing? |
This are two different things: Work on an open source project and working on foreign code. You just did the latter. Work on a project means communicating with maintainers and consider how to get your modifications merged. You should have mentioned that you are not interested in merging the code and real contributions. This would have saved me some resources.
If you commit into the |
Removed
Fix #1578