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

A couple of minor patches to Jamulus.pro #3010

Merged
merged 2 commits into from
Feb 14, 2023

Conversation

pljones
Copy link
Collaborator

@pljones pljones commented Feb 13, 2023

Short description of changes

  • Qt Creator 5 "built in" MingW is "pre-Win8" and needs _WIN32_WINNT=0x0600 to get inet_pton to work.
  • When trying to build a distributable, I discovered quite a lot of the files aren't mentioned in DIST_FILES (so they don't appear as "part of the project" to Qt Creator)

CHANGELOG: Internal: Improve Qt Creator 5 compliance of Jamulus.pro

Context: Fixes an issue?

Minor developer issue with development tooling.

Does this change need documentation? What needs to be documented and how?

No.

Status of this Pull Request

I had to make the changes to get my fresh new Qt Creator install working fully.

What is missing until this pull request can be merged?

Review.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see self-requested a review February 13, 2023 21:22
@ann0see ann0see added this to the Release 3.10.0 milestone Feb 13, 2023
@@ -81,6 +81,7 @@ win32 {
DEFINES += NOMINMAX # solves a compiler error in qdatetime.h (Qt5)
RC_FILE = src/res/win-mainicon.rc
mingw* {
DEFINES += _WIN32_WINNT=0x0600 # solves missing inet_pton in CSocket::SendPacket
Copy link
Member

Choose a reason for hiding this comment

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

Is this fixed with the Qt6 creator? If that's the case, I'd add it here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It all got too complicated to try... Ideally, both should work together.

@ann0see
Copy link
Member

ann0see commented Feb 14, 2023

Approved on the base that it's an improvement in a non critical area (doesn't seem to break the CI which uses a different process)

@pljones
Copy link
Collaborator Author

pljones commented Feb 14, 2023

@ann0see once the JACK build is green, can you force the merge if you're okay to do it? (Rather than me forcing my own change...)

@ann0see
Copy link
Member

ann0see commented Feb 14, 2023

I have the feeling that something is wrong on the JACK side. At least it seems as if it hangs now, even though it didn't two weeks ago.

@ann0see
Copy link
Member

ann0see commented Feb 14, 2023

Now the JACK build is ok.

@ann0see ann0see merged commit 1e05169 into jamulussoftware:main Feb 14, 2023
@pljones pljones deleted the patch/jamulus-pro-fixes branch February 15, 2023 17:36
@pljones pljones self-assigned this Jul 29, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants