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

Update Windows build instructions & contributing docs #4858

Merged
merged 34 commits into from
Oct 9, 2023

Conversation

GongBingWong
Copy link
Contributor

Description

Based on my experience as a first time contributor, I added what I thought would be useful to know for other first time contributors.

I updated the 'BUILDING_ON_WINDOWS.md' file to include more steps for Qt, included how to add to PATH for Qt and conan 2, and added 'cd ..' as a step to build.

I updated the 'CONTRIBUTING.md' file (not really sure where to put general non-code related contribution info) to include what I learned with my contribution (the need to create a specific branch instead of using main, and also to update the 'CHANGELOG.md' file).

BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
Based on Felanbird's observation.
Comment on lines 98 to 110
<details>
<summary>How to add conan 2 to PATH</summary>

1. Type "path" in the start menu and click on the "Edit the system environment variables" result that shows up (this should open up the 'System Properties' window with the 'Advanced' tab selected).
2. Press on the "Environment Variables..." button near the bottom right of the 'System Properties' window to open up the 'Environment Variables' window.
3. Within 'Environment Variables', look at the bottom half of the window where it lists 'System variables' and double click on the variable named 'Path' to open up the 'Edit environment variable' window.
4. Press the 'New' button near the top right to create a new environment variable.
5. Open up your terminal with the Visual Studio environment variables (e.g. `x64 Native Tools Command Prompt for VS 2022`) and type "where conan" to find the file path (the folder that contains the conan.exe) to add.
6. Add conan 2's file path (e.g. `C:\Users\willg\AppData\Roaming\Python\Python311\Scripts`) to the blank text box that shows up.
7. Press the "OK" button.

</details>

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<details>
<summary>How to add conan 2 to PATH</summary>
1. Type "path" in the start menu and click on the "Edit the system environment variables" result that shows up (this should open up the 'System Properties' window with the 'Advanced' tab selected).
2. Press on the "Environment Variables..." button near the bottom right of the 'System Properties' window to open up the 'Environment Variables' window.
3. Within 'Environment Variables', look at the bottom half of the window where it lists 'System variables' and double click on the variable named 'Path' to open up the 'Edit environment variable' window.
4. Press the 'New' button near the top right to create a new environment variable.
5. Open up your terminal with the Visual Studio environment variables (e.g. `x64 Native Tools Command Prompt for VS 2022`) and type "where conan" to find the file path (the folder that contains the conan.exe) to add.
6. Add conan 2's file path (e.g. `C:\Users\willg\AppData\Roaming\Python\Python311\Scripts`) to the blank text box that shows up.
7. Press the "OK" button.
</details>

As noted by the instruction above, the default settings automatically setup PATH to recognize conan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. I didn't even notice the "default setting". xD

Also do you know if the 'where conan' command only works if conan 2 is in PATH?

If so, maybe it could be used to verify conan 2 is in PATH.

BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
@@ -2,6 +2,10 @@

This is a set of guidelines for contributing to Chatterino. The goal is to teach programmers without a C++ background (java/python/etc.), people who haven't used Qt, or otherwise have different experience, the idioms of the codebase. Thus we will focus on those which are different from those other environments. There are extra guidelines available [here](https://hackmd.io/@fourtf/chatterino-pendantic-guidelines) but they are considered as extras and not as important.

### General (non-code related) guidelines for contributing to Chatterino
- Make a specific branch for your pull request instead of using the master, main, or mainline branch. This allows you to better manage your workflow and avoid conflicts with Chatterino's main branch name.
- Update the CHANGELOG.md file to include bug fixes and features that you implemented. The number behind the changelog entry indicates the number of the pull request.
Copy link
Collaborator

Choose a reason for hiding this comment

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

While it is true the changelog check will cry without a changelog, I don't think first time contributors should feel like they need to come up with a changelog entry, where applicable we'll just write one for you if one isn't included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, should that info be mentioned then?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say just omit the changelog stuff, the branch thing is handy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
@@ -82,19 +93,33 @@ Note: This installation will take about 200 MB of disk space.

Install [conan 2](https://conan.io/downloads.html) and make sure it's in your `PATH` (default setting).

<details>
<summary>How to add conan 2 to PATH</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

This adds python's scripts to PATH. Maybe this should read something like "Installing with pip" and include the pip install. Furthermore, I thought default python installations would add themselves to PATH by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, maybe its pip specific. I don't know if default python installations add themselves to PATH by default, I think I manually added conan 2 to PATH (don't remember if it was there before).

I'm probably just going to commit Felanbird's suggestion to remove it after I ask about something.

The reason I haven't committed it yet because I'm not sure if I should resolve a conversation (by doing a commit) when I still have a question about a related topic.

Thanks to Felanbird and Nerixyz.
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
@@ -2,6 +2,10 @@

This is a set of guidelines for contributing to Chatterino. The goal is to teach programmers without a C++ background (java/python/etc.), people who haven't used Qt, or otherwise have different experience, the idioms of the codebase. Thus we will focus on those which are different from those other environments. There are extra guidelines available [here](https://hackmd.io/@fourtf/chatterino-pendantic-guidelines) but they are considered as extras and not as important.

### General (non-code related) guidelines for contributing to Chatterino
- Make a specific branch for your pull request instead of using the master, main, or mainline branch. This allows you to better manage your workflow and avoid conflicts with Chatterino's main branch name.
- Update the CHANGELOG.md file to include bug fixes and features that you implemented. The number behind the changelog entry indicates the number of the pull request.
Copy link
Member

Choose a reason for hiding this comment

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

I'd say just omit the changelog stuff, the branch thing is handy

@@ -28,15 +29,25 @@ Notes:
#### When prompted which components to install:

1. Unfold the tree element that says "Qt"
2. Unfold the top most tree element (latest stable Qt version, e.g. `Qt 5.15.2`)
2. Unfold the top most tree element (latest stable Qt version, e.g. `Qt 6.5.3`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is a bit late, but Qt 6.6.0 will be out tomorrow (most likely), and we noticed a bug in Qt 6.5.3 [not in 6.6], so I think it would be better to use 6.6.0 here, but it's not that big of a deal.

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 ok with this not being the absolute latest or correct version since it's an example - as long as it's Qt 6 it'll be close enough

Copy link
Contributor

@Nerixyz Nerixyz left a comment

Choose a reason for hiding this comment

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

When reading the entire file, I noticed that we're saying Qt 5.12 and up are supported, even though on Windows, we're only supporting Qt 5.15 and up (technically 5.12 should still compile though).

  • Installing the latest stable Qt version is advised for new installations, but if you want to use your existing installation please ensure you are running Qt 5.12 or later.

BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
BUILDING_ON_WINDOWS.md Show resolved Hide resolved
@pajlada pajlada changed the title Add better documentation for new contributors Update Windows build instructions & contributing docs Oct 9, 2023
BUILDING_ON_WINDOWS.md Outdated Show resolved Hide resolved
@pajlada pajlada enabled auto-merge (squash) October 9, 2023 18:19
@pajlada pajlada merged commit d03151d into Chatterino:master Oct 9, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants