Skip to content
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

feat: add link to the logs folder in borg warnings #1609

Merged
merged 20 commits into from
Mar 12, 2023

Conversation

diivi
Copy link
Contributor

@diivi diivi commented Feb 24, 2023

Description

Adds a link to the vorta logs folder on the user's device in the progressText qLabel.
Please let me know in what state I should leave the strings for translation, thanks.

Related Issue

#1486

Motivation and Context

Easier to open the logs folder without having to look for it (better UX).

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING guide.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

@diivi diivi marked this pull request as ready for review February 25, 2023 10:27
Copy link
Collaborator

@real-yfprojects real-yfprojects left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What purpose serves the last commit? You don't have to modify any translation files. That's all done automatically in preparation of a release. There is a rule for that in our make file.

You will need to use translate instead of trans_late else the translations won't be loaded at runtime. Please tell me if any part of the contributing docs is unclear.

src/vorta/borg/check.py Outdated Show resolved Hide resolved
src/vorta/application.py Outdated Show resolved Hide resolved
real-yfprojects
real-yfprojects previously approved these changes Mar 2, 2023
Copy link
Collaborator

@real-yfprojects real-yfprojects left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my local testing Qt won't recognise the translate statements spanning over multiple lines

Luckily it seems to work again/with this PR.

@real-yfprojects real-yfprojects requested a review from m3nu March 2, 2023 16:56
@diivi diivi requested review from ThomasWaldmann and removed request for m3nu March 5, 2023 18:13
ThomasWaldmann
ThomasWaldmann previously approved these changes Mar 5, 2023
Copy link
Collaborator

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

src/vorta/borg/create.py Show resolved Hide resolved
@real-yfprojects
Copy link
Collaborator

@diivi Can you rebase onto master and adjust this PR. LOG_DIR is of type pathlib.Path now.

@diivi diivi requested review from real-yfprojects and removed request for m3nu March 11, 2023 10:32
Copy link
Contributor

@m3nu m3nu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as advertised. 👏

Copy link
Collaborator

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess the checking for rc 0, 1, 2 could be improved, but that is not in scope of this PR.

0 == OK
1 == Warning
2 == Error

Any messages derived from this should use a corresponding wording.

@real-yfprojects real-yfprojects merged commit 6bc5321 into borgbase:master Mar 12, 2023
diivi added a commit to diivi/vorta that referenced this pull request Apr 11, 2023
In case a borg job finishes with warning, vorta will display it and tell the user to have a look in the logs. This adds a clickable link to the log message that opens the default file explorer at the log location.

* src/vorta/application.py (VortaApp.check_failed_response): Improve wording of warning message and link logs.

* src/vorta/borg/create.py (BorgCreateJob.process_result): Link logs.
* src/vorta/borg/compact.py (BorgCompactJob.finished_event): ^^
* src/vorta/borg/check.py (BorgCheckJob.finished_event): ^^

* src/vorta/assets/UI/mainwindow.ui : Enable `openExternalLinks` for `progressText` label.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Say where the logs are when telling about warnings in borg execution
4 participants