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

Improve COMPILING, CONTRIBUTING and TRANSLATING files #2561

Merged

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Mar 28, 2022

Short description of changes

Updates mainly COMPILING and CONTRIBUTING files. Some major and minor changes have been introduced.

CHANGELOG: Documentation: Rewrite some parts of COMPILING.md and CONTRIBUTING.md to stay up to date and enable clearer contribution guidelines

Context: Fixes an issue?

Fixes: #2553
Fixes: #2289

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

Maybe an updated contribution page on the website.

Status of this Pull Request

Ready for a hopefully not too long discussion.

What is missing until this pull request can be merged?

Review by @pgScorpio

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want (n/a)
  • My code follows the style guide (n/a)
  • 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 closed this Mar 28, 2022
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from 735f9ea to 7593f64 Compare March 28, 2022 21:05
@ann0see ann0see reopened this Mar 28, 2022
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from f053327 to df47b87 Compare March 28, 2022 21:08
@ann0see

This comment was marked as outdated.

Copy link
Contributor

@pgScorpio pgScorpio left a comment

Choose a reason for hiding this comment

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

Already much better but:

I think we should note the minimum, maximum and preferred version of Qt to use.

Also note to install the required 'opensource' version and not to use the 'universal installer'
Also the given Qt link will most likely lead to installing the latest Qt version which will not work. Maybe better directly link to the suitable versions at https://download.qt.io/official_releases/qt/ ?

Qt for macOS homebrew link does not work anymore (Should be brew install qt@5 ??) And why use homebrew ? better versions are available via the opensource installer. (And homebrew gave a lot of issues for me, finally still ending up with an incomplete Qt version.)
Qt for iOS: When building on macOS I think it's better to install the correct macOS version with the official opensource installer and then to make sure to also check the iOS option when installing.

Same goes for XCode: The given link (app store) will download the latest version, which won't work for Jamulus. Better state which version (11 ?) to use and use the link https://developer.apple.com/download/all/q=Xcode ?

Build on windows: Edit the $QtCompile32 and $QtCompile64 variables if needed ??
I'm still wondering why this is needed, since it gives rebase problems every time, why not use command line options and/or environment variables ? (We should create an issue to modify the script?)
For just debugging/testing there are also simpler options when using Visual Studio... (qmake -tp vc Jamulus.pro and simply build in VS)

Note: "If ? have a free Apple Developer Account", You must have, otherwise you couldn't download the needed software either.

@ann0see
Copy link
Member Author

ann0see commented Mar 29, 2022

"via choco"?

@jujudusud what would you write?

@ann0see
Copy link
Member Author

ann0see commented Mar 29, 2022

I think we should note the minimum, maximum and preferred version of Qt to use.

At the moment it's probably not clear for us either. We basically set the lowest version to Qt5.0 (if that exists) and the highest one to Qt 5.15.2/5.15.3 once Qt6 support is ready, we'd probably raise the maximum supported version.

Also note to install the required 'opensource' version and not to use the 'universal installer'

I thought that's already documented - at least for Windows. Could you please click on the lines you'd want to add it and - in the editor - give a suggestion on what you'd want to add. See how to do this here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/commenting-on-a-pull-request

Qt for macOS homebrew link does not work anymore (Should be brew install qt@5 ??) And why use homebrew ? better versions are available via the opensource installer. (And homebrew gave a lot of issues for me, finally still ending up with an incomplete Qt version.)

Good to know it doesn't work anymore. I thought it's much easier to get Qt via brew since you'd not need the somewhat bloated Qt installer (with login).

Same goes for XCode: The given link (app store) will download the latest version, which won't work for Jamulus.

For me the latest version worked well?

Build on windows: Edit the $QtCompile32 and $QtCompile64 variables if needed ??
I'm still wondering why this is needed, since it gives rebase problems every time, why not use command line options and/or environment variables ? (We should create an issue to modify the script?)

Probably worth opening another issue (and if possible setting them to some default folders...)

No. Will edit the file

For just debugging/testing there are also simpler options when using Visual Studio... (qmake -tp vc Jamulus.pro and simply build in VS)

Probably yes. You're much more experienced with the VS setup and I don't know much about how to set it up (as I develop mainly on Linux)

Note: "If ? have a free Apple Developer Account", You must have, otherwise you couldn't download the needed software either.

There's a distinction between Apple ID without Developer account (= allows you to download Xcode), Free developer account (you'll need to register your normal Apple ID or at least accept some terms somewhere to get a developer account) (= allows you to actually develop apps and run them on iOS devices) and the $99 one (which emlyn uses to sign the build which allows you to publish apps on the AppStore/sign them). Probably we need to clarify it a bit more, but I thought it's clear like that.

@jujudusud
Copy link
Member

"via choco"?

@jujudusud what would you write?

You wrote "via coco" instead of "via choco".

@ann0see
Copy link
Member Author

ann0see commented Mar 29, 2022

Ok. Fixed.

@ann0see
Copy link
Member Author

ann0see commented Mar 29, 2022

Concerning the Qt variables, I think they don't need to be changed at all:

Just give the correct path to the Qt build.

COMPILING.md Outdated Show resolved Hide resolved
COMPILING.md Outdated
@@ -26,6 +33,8 @@ On Ubuntu-based distributions 18.04+, Debian 9+, and Raspberry Pi OS:
- qttools5-dev-tools
- libjack-jackd2-dev

**Note:** The exact dependencies might be different for older distributions. See [this comment by softins](https://github.com/jamulussoftware/jamulus/pull/2267#issuecomment-1022127426)

### On Fedora 33+

- qt5-qtdeclarative-devel
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering why qt5-qtdeclarative-devel should be the correct package. As far as I understand, it's mainly related to QML, which we don't use. Maybe it's a package which happens to pull in the right dependencies for us, but this should probably be re-checked.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know about Fedora. Someone else should have a look at that

COMPILING.md Outdated Show resolved Hide resolved
COMPILING.md Show resolved Hide resolved
COMPILING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from d37afc3 to e2a4c14 Compare March 30, 2022 20:55
@ann0see ann0see requested a review from hoffie March 30, 2022 21:17
@ann0see ann0see linked an issue Mar 30, 2022 that may be closed by this pull request
@pgScorpio
Copy link
Contributor

Probably yes. You're much more experienced with the VS setup and I don't know much about how to set it up (as I develop mainly on Linux)

Just let them run:
qmake -tp vc Jamulus.pro
and, after that, they can open the Jamulus.vcxproj file in Visual Studio
But we should add a note that they will have to repeat the qmake -tp vc Jamulus.pro after Jamulus.pro has changed.

In visual studio they will be able to build a debug or release version for testing and they will only need the scripts if they want to create the installer.

P.S. Why do we use Powershell scripts for this in windows ?? Wouldn't be running sh scripts from Git Bash be more consistent ??
(the sh script can still run a Powershell script whenever absolutely necessary , but a sh script can use other common sh scripts, as used for all other platforms, as well.)

@ann0see ann0see force-pushed the patch/updateCompileContribute branch 2 times, most recently from 3c8c6d7 to f08825e Compare March 31, 2022 20:56
@ann0see
Copy link
Member Author

ann0see commented Apr 1, 2022

P.S. Why do we use Powershell scripts for this in windows ?? Wouldn't be running sh scripts from Git Bash be more consistent ??

I don't think windows ships .sh support by default?

@ann0see
Copy link
Member Author

ann0see commented Apr 1, 2022

Just a small note/thought/question to @pgScorpio which came up to me right now: Are you used to developing mostly in a small team where everyone knows what the other people do or alone? I think some confusion about git might be the distributed nature of Free/OpenSource Software. I think the way of development here is very different to what I assume you're used to. So it's probably the source of confusion?

@pgScorpio
Copy link
Contributor

I don't think windows ships .sh support by default?

No, but you will need git anyway and so you will also have Git Bash which does support it.
So why should we need to open two different consoles ?
I even already use a bash script to run deploy_windows.ps1

#!/bin/bash
powershell ./windows/deploy_windows.ps1

just because most of the time I already have Git Bash open and I'm too lazy to open a powershell too ;=)

@pgScorpio
Copy link
Contributor

@ann0see

Just a small note/thought/question to @pgScorpio which came up to me right now: Are you used to developing mostly in a small team where everyone knows what the other people do or alone? I think some confusion about git might be the distributed nature of Free/OpenSource Software. I think the way of development here is very different to what I assume you're used to. So it's probably the source of confusion?

Indeed, most of the time I developed in teams with around 10 developers (most of the time all at the same location), and I am very used to working with PVCS and Microsoft VSS. So the confusion is because these are quite different from Git with a lot of same terminology, like Fetch and Checkout, but doing something completely different. Also the 3 stage archiving of Git makes branching, merging and rebase-ing is quite a lot more complicated and very confusing for me after using the other version control systems for over 25 years. But I'm learning... It's just a bit frustrating that in the past weeks I'm spending more time on Git problems than on actual coding. So I still would gladly accept your help with Git and on correcting some of my mistakes. (Could you PM me on that?)

@ann0see
Copy link
Member Author

ann0see commented Apr 1, 2022

GitHub has no PM feature, but feel free to E-Mail me: https://github.com/ann0see/#hi-there--im-ann0see

@pgScorpio
Copy link
Contributor

@ann0see

GitHub has no PM feature, but feel free to E-Mail me: https://github.com/ann0see/#hi-there--im-ann0see

?? I don't see any email address there, but mine should be in my profile I think. (if not: pg_Scorpio@hotmail.com)

@ann0see
Copy link
Member Author

ann0see commented Apr 2, 2022

Sent you a mail this morning.

@pgScorpio
Copy link
Contributor

pgScorpio commented Apr 2, 2022 via email

@ann0see
Copy link
Member Author

ann0see commented Apr 2, 2022

Strange. I've sent it via my own mailserver now. Maybe hotmail dislikes my E-Mail provider (that seems to happen quite often). Nevertheless, we can also meet today?

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from 1a69e95 to e452695 Compare April 17, 2022 15:17
@ann0see ann0see reopened this Apr 17, 2022
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from 5cedfcc to f3c9648 Compare April 17, 2022 15:24
COMPILING.md Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the patch/updateCompileContribute branch 2 times, most recently from 2f29817 to 97d56e6 Compare April 17, 2022 17:04
CONTRIBUTING.md Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the patch/updateCompileContribute branch 4 times, most recently from d38b905 to 28c3b34 Compare April 18, 2022 19:04
@ann0see
Copy link
Member Author

ann0see commented Apr 18, 2022

@gilgongo ran an AI over the file to make it sound a bit more natural: https://quillbot.com/

Could you please re-check?

@ann0see ann0see force-pushed the patch/updateCompileContribute branch 4 times, most recently from bae4930 to 76bec7a Compare April 18, 2022 19:14
Co-Authored-By: Jonathan <4561747+gilgongo@users.noreply.github.com>
Co-Authored-By: Christian Hoffmann <christian@hoffie.info>
Co-Authored-By: Peter L Jones <pljones@users.noreply.github.com>
@ann0see ann0see force-pushed the patch/updateCompileContribute branch from 76bec7a to 425338f Compare April 18, 2022 19:16
@ann0see ann0see requested a review from hoffie April 19, 2022 21:13
@ann0see

This comment was marked as outdated.

@ann0see
Copy link
Member Author

ann0see commented Apr 30, 2022

@gilgongo please merge if this is ok for you.

@ann0see ann0see changed the title Improve COMPILING and CONTRIBUTING files Improve COMPILING, CONTRIBUTING and TRANSLATING files Apr 30, 2022
CONTRIBUTING.md Outdated Show resolved Hide resolved
@gilgongo gilgongo merged commit 282306d into jamulussoftware:master May 2, 2022
@ann0see ann0see deleted the patch/updateCompileContribute branch May 2, 2022 07:42
@hoffie hoffie added this to the Release 3.9.0 milestone Jun 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants