Contributing: Apply suggestion from reviewers

This commit is contained in:
Samantaz Fox 2024-07-21 18:39:59 +02:00
parent 91fdbf6033
commit 69e2f37539
No known key found for this signature in database
GPG Key ID: F42821059186176E

View File

@ -14,7 +14,7 @@ with Invidious. Here is a summary:
to help us reach more users around the globe. to help us reach more users around the globe.
* [Hosting an instance](#hosting-an-instance): You're a sysadmin and have some time and server * [Hosting an instance](#hosting-an-instance): You're a sysadmin and have some time and server
ressources to spare, with some prior experience with invidious. ressources to spare, with some prior experience with Invidious.
* [Testing](#testing): You're an advanced user, who is already running their own instance, is not * [Testing](#testing): You're an advanced user, who is already running their own instance, is not
worried of compiling Invidious from source, and wants to help us test bug fixes or new features. worried of compiling Invidious from source, and wants to help us test bug fixes or new features.
@ -29,34 +29,37 @@ with Invidious. Here is a summary:
We use the [Github issue tracker](https://github.com/iv-org/invidious/issues) to track and manage We use the [Github issue tracker](https://github.com/iv-org/invidious/issues) to track and manage
bug reports, feature requests and improvements. bug reports, feature requests and improvements.
**Note: Before opening any kind of issue, make sure to search on the tracker **Note: Before opening any kind of issue, make sure to search on the tracker with various keywords
with various keywords to verify that no other similar issue have already been to verify that no other similar issue have already been opened (and/or closed).**
opened (and/or closed).**
In order for everyone to be able to understand eachother, all exchanges are done in English. In order for everyone to be able to understand eachother, all exchanges are done in English.
You can obviously use a translator if needed. You can obviously use a translator if needed, but if you do, please mention it in your message, so
that we can be aware of it, and respond accordingly: nuance often gets lost in translation.
Please be polite and respectful in your exchanges. Remember that we're volunteers providing that Please be polite and respectful in your exchanges. Remember that we're volunteers providing that
service for free. We don't have the time nor energy to deal with bad manners. service for free. We don't have the time nor energy to deal with bad manners.
Users who (understandably) do not want to use GitHub can [contact us](https://invidious.io/contact/)
with the required details. If you can write something we can directly paste into a GitHub issue that
would be perfect!
### Bug reports ### Bug reports
The most common case is that you ended up on a page saying "you encountered a The most common case is that you ended up on a page saying "you encountered a bug in Invidious"
bug in Invidious" while you were browsing a channel or watching a video. while you were browsing a channel or watching a video.
In that case, you should open a "bug report". Please include as many details as possible, so Before anything else, **make sure to test a few other instances**, just to check that the
that we can easily reproduce your problem on our side.
Before opening your issue, **make sure to test a few other instances**, just to check that the
problem you're facing is not temporary (E.g an overloaded instance, a network outage, etc.) or problem you're facing is not temporary (E.g an overloaded instance, a network outage, etc.) or
caused by configuration error on your side. caused by a configuration error on that specific instance.
If a bug report already exists for your problem, you can add a comment with more details, but make If a bug report already exists for your problem, you can add a comment with more details, but make
sure that it's useful and adds value to the discussion. If 20 people did that already, it might not sure that it's useful and adds value to the discussion. If 20 people did that already, it might not
be relevant to post a new comment. be relevant to post a new comment.
Here is a non-exhaustive list of details that will help us: Otherwise, you should open a "bug report". Please include as many details as possible, so that we
can easily reproduce your problem on our side. Here is a non-exhaustive list of details that will
help us:
* **A clear and concise list of steps to reproduce the problem** * **A clear and concise list of steps to reproduce the problem**
* A link to the page where the bug happened * A link to the page where the bug happened
@ -102,7 +105,7 @@ Invidious is translated in many languages using Weblate:
https://hosted.weblate.org/projects/invidious/. https://hosted.weblate.org/projects/invidious/.
We recommend creating an account or connecting with one of the other authentication providers that We recommend creating an account or connecting with one of the other authentication providers that
Weblate supports (Github, GitLab, Google, etc..) for a better experience. Weblate supports (Github, GitLab, etc..) for a better experience.
We also accept translation updates using Pull requests, but please be aware that it represents more We also accept translation updates using Pull requests, but please be aware that it represents more
work for us to merge those. work for us to merge those.
@ -114,9 +117,9 @@ work for us to merge those.
Another way to contribute to invidious is to host a [public instance] Another way to contribute to invidious is to host a [public instance]
(https://instances.invidious.io/). (https://instances.invidious.io/).
To do so, you need a server (either a VPS or dedicated) with To do so, you need a server with [enough ressources]
[enough ressources](https://docs.invidious.io/installation/#hardware-requirements) to handle the (https://docs.invidious.io/installation/#hardware-requirements) to handle the load. You will also
load. You'll also need a domain name with a valid TLS certificate (e.g provided by Let's Encrypt). need a domain name with a valid TLS certificate.
Then, if your instance follows the Then, if your instance follows the
[rules listed here](https://docs.invidious.io/instances/#rules-to-have-your-instance-in-this-list), [rules listed here](https://docs.invidious.io/instances/#rules-to-have-your-instance-in-this-list),
@ -125,27 +128,43 @@ you can request your instance to be added to the list by
Once you've filled the form with your instance's informations, your instance will be added to our Once you've filled the form with your instance's informations, your instance will be added to our
uptime monitor. From there, a probatory period of 30 days will start, to make sure that you can uptime monitor. From there, a probatory period of 30 days will start, to make sure that you can
keep your instance online and up to date. Finally, your instance will be added to the list. We'll keep your instance online and up to date. Finally, your instance will be added to the list. After
ask you to join our Matrix room, so that we can inform you of important updates and exchange with that We will invite you to a dedicated Matrix room for instance maintainers.
other maintainers.
Joining this room is not mandatory but strongly recommended, as we use it to broadcast information
about important updates, and you can also exchange with other maintainers.
## Testing ## Testing
Once reviewed, the new features or bug fixes must be tested before being merged. In general, this New features and bug fixes must be tested before being merged.
can be achieved by running the following commands (change the PR number as required):
Once reviewed, pull requests that need to be tested will be labelled as [`need-testing`]
(https://github.com/iv-org/invidious/pulls?q=is%3Apr+is%3Aopen+label%3Aneed-testing). When one PR is
deployed on our test instance, it will be marked accordingly as [`in-testing`]
(https://github.com/iv-org/invidious/pulls?q=is%3Apr+is%3Aopen+label%3Ain-testing).
If you have prior knowledge of Invidious, that's great, but if you don't, that's also okay! We have
a Matrix room (also bridged to IRC) that you can join to get help. We'll do our best to help you
getting started with the project.
In general, testing these changes yourself can be achieved using the commands below (change the PR
number as required):
```bash ```bash
# Clone the repository
git clone https://github.com/iv-org/invidious git clone https://github.com/iv-org/invidious
cd invidious cd invidious
wget "https://github.com/iv-org/invidious/pull/4439.diff"
git apply 4439.diff # Fetch the code to a new branch (here "testing") and make it the current working tree
# Don't forget to change the PR number!
git fetch upstream pull/1234/head:testing
git checkout testing
# Finally, run a test instance using docker
docker compose up docker compose up
``` ```
If you have prior knowledge of Invidious, that's great, but otherwise feel free to get in touch
with us on Matrix or IRC. We'll do our best to give you a better understanding of the project.
Once you have deployed the patch on your test instance, try the changes mentionned in the pull Once you have deployed the patch on your test instance, try the changes mentionned in the pull
request. Often times, the linked issue might contain examples of channels/comments/videos impacted. request. Often times, the linked issue might contain examples of channels/comments/videos impacted.
Definitely use those to check that the behavior you see is the expected one. After that, add a Definitely use those to check that the behavior you see is the expected one. After that, add a
@ -159,12 +178,46 @@ changes) and code snippets (for API changes) as needed.
Code contributions are handled through Code contributions are handled through
[Github's Pull Requests (PRs)](https://github.com/iv-org/invidious/pulls). [Github's Pull Requests (PRs)](https://github.com/iv-org/invidious/pulls).
Invidious' backend is developed in Crystal, a compiled language inspired from Ruby. The frontend
is developed using Crystal's own templating engine (ECR) with some vanilla JS and CSS.
Invidious follows more or less closely the [Crystal coding convention] ### Generalities
#### Server side
The server part of Invidious is developed in [Crystal](https://crystal-lang.org/), a compiled
language inspired by Ruby. The HTML templates are generated at compile time from the ECR files
present in the `src/invidious/views` folder, meaning that you need to re-compile Invidious after
changing those.
Regarding coding style, Invidious tries to follow the [Crystal coding convention]
(https://crystal-lang.org/reference/latest/conventions/coding_style.html#directory-and-file-names), (https://crystal-lang.org/reference/latest/conventions/coding_style.html#directory-and-file-names),
except for the "Directory and File Names" sections. as closely as possible. Most of the rules listed here will be enforced if you run `make format`.
We also use [Ameba](https://github.com/crystal-ameba/ameba), a static code analysis tool for Crystal.
Ameba is part of our CI/CD pipeline, but it is recommended to run it locally before pushing you
change and making a PR, like so:
```sh
# Make sure to have all dependencies installed, including the development ones
# (this command only needs to be run once)
shards install
# Run ameba
./bin/ameba
```
#### Client side
The client side of Invidious is developped with some basic ("vanilla") JavaScript and CSS.
No complex JS framework or tooling is required (e.g NPM).
The only dependencies Invidious has is [VideoJS](https://github.com/videojs/video.js/) plus some of
its plug-ins. VideoJS is automatically downloaded when you run `make`.
#### Other
If you need to edit files the `locales/` directory, please make sure to keep the indentation as four
spaces. Any other identation will break Weblate, our translation tool (See #Translations above).
### Contributing code ### Contributing code