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

Merge release-7.5 into devel #5427

Merged
merged 12 commits into from
Jul 1, 2020
Merged

Merge release-7.5 into devel #5427

merged 12 commits into from
Jul 1, 2020

Conversation

devos50
Copy link
Contributor

@devos50 devos50 commented Jul 1, 2020

No description provided.

devos50 and others added 12 commits July 1, 2020 10:47
We used _run_once to ensure that some exceptions are raised (and do not crash Tribler) before asserting that the exception handler has not been called. However, the behaviour of _run_once is not always such that it immediately cleans the events in the event loop. This results in unexpected behaviour (at least on Mac with Python 3.8) where the test starts to tear down before the exceptions are thrown and failing the test.
This fixes an obscure bug on Mac, where the Tribler core would crash if VLC is loaded and instantiated before the subprocess is created. It should also speedup the overall Tribler startup since the VLC libs are now only loaded after Tribler has started.
@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 3 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.4% 0.4% Duplication

@devos50 devos50 marked this pull request as ready for review July 1, 2020 08:59
@devos50 devos50 requested review from ichorid and qstokkink July 1, 2020 08:59
@devos50 devos50 merged commit fd748af into Tribler:devel Jul 1, 2020
@devos50 devos50 deleted the devel_update branch July 1, 2020 09:11
@ichorid
Copy link
Contributor

ichorid commented Jul 1, 2020

изображение

🤦

the merge must be done directly from release-7.5 branch into devel branch, by hand in git. And pushed manually through the command line (with checking it as a PR of course). Otherwise, history becomes a complete mess and you can't merge stuff without conflicts anymore.

That's how it should look:
изображение

@devos50
Copy link
Contributor Author

devos50 commented Jul 1, 2020

Ok, missed that one. Even though I really don't like to push directly to devel, if that is the best practice I will do it like that next time.

@qstokkink
Copy link
Contributor

if that is the best practice

The official Git best practice is (see https://git-scm.com/book/en/v2/Distributed-Git-Distributed-Workflows#ch05-distributed-git):
"make it as easy on you and the project maintainer as possible"

The Atlassian (Bitbucket) best practice (see https://www.atlassian.com/git/tutorials/merging-vs-rebasing) is:
"If you would prefer a clean, linear history free of unnecessary merge commits, you should reach for git rebase [...] if you want to preserve the complete history of your project and avoid the risk of re-writing public commits, you can stick with git merge"

I prefer the risk-averse approach in IPv8, but the Tribler clean-branch approach is also excellently documented by Atlassian. But, as usual: "it depends" is the answer to what your project-specific best practice should be. We've already has lengthy discussions on this topic 😃

@qstokkink
Copy link
Contributor

Oh, by the way, I created issue #5409 specifically for this reason. If the branch merge magic is done automatically, there can be no mistakes with merging vs. rebasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants