Closed heldersepu closed 5 months ago
Hi @heldersepu, thank you for contributing to Tribler! 🚀
This PR was created by an outside collaborator, so some checks were not run for security reasons.
To trigger the full set of checks, any member of the Tribler team can add the label PR: safe to check
to the current PR.
Hi, @heldersepu!
Thank you for your improvement. I see a lot of errors in the checks for this PR. However, they are not related to the changes in this PR and are caused by something else. I'll figure out the reasons for them, fix them, and then return to this PR.
@heldersepu, could you please rebase your branch on the most recent main
? The issues seem to be fixed.
@drew2a done
Thank you, @heldersepu!
Two comments:
main
branch into yours instead of rebasing your branch onto main
. It's fine for now, but perhaps when the PR is ready, it would be nice to rebase it before merging (though it's not necessary).
When do you use git rebase instead of git merge?test_clipboard
causes the GUI tests to fail. You should probably mock QApplication
or some parts of it.... but Merging is so much easier... github gives us a button they call [Sync fork] on my fork is just one click and done, why will anyone still use rebase
Looking into the failing tests, when I started I noticed the copy_to_clipboard
did not have any tests, now I see why.
I'm just going to remove that test, making sure copy/paste works is counter productive
why will anyone still use rebase
It gives you a cleaner commit history. If you spend a lot of time doing bug fixing and figuring out when and how an error was introduced, you start to appreciate the clarity of a well-maintained commit history :)
@heldersepu, thank you for your effort and cooperation.
It really is my pleasure @drew2a ... I've been using Tribbler for a while and always found a few quirks that could be improved, but never put the time to read the code; I just ran into a bug with the http_port used for the api that I could not leave alone: https://github.com/Tribler/tribler/issues/8007 that one unfortunately I could not fix, there seems to be much more discussion that we need to settle before a permanent code fix is applied
A simple change to add magnet to the downloads with paste (
QKeySequence("Ctrl+v")
) this speeds up the many click steps, ideally more options will be added in the future..