-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Update CONTRIBUTING.md development process #13635
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
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,12 @@ | ||||||||||||
| ### Please do **not** open pull requests for *new features* now, as we are planning to rewrite large chunks of the code. Only bugfix PRs will be accepted. More details will be announced soon! | ||||||||||||
| ### Please do **NOT** open pull requests for *new features* now, as we are currently refactoring the codebase. Only bugfix PRs or refactors will be accepted. | ||||||||||||
|
|
||||||||||||
| NewPipe contribution guidelines | ||||||||||||
| =============================== | ||||||||||||
|
|
||||||||||||
| **Please make every effort to adhere to these guidelines** | ||||||||||||
| It is in both your and our best interests that you follow the process so everything can be done quickly and in order. | ||||||||||||
| If you believe there's anything in these guidelines that is inefficient, missing, unclear, or can otherwise be improved please let us know and we will address it | ||||||||||||
|
|
||||||||||||
| ## AI policy | ||||||||||||
|
|
||||||||||||
| * Using generative AI to develop new features or making larger code changes is generally prohibited. Please refrain from contributions which are heavily depending on AI generated source code because they are usually lacking a fundamental understanding of the overall project structure and thus come with poor quality. However, you are allowed to use gen. AI if you | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the period after "gen". |
||||||||||||
|
|
@@ -18,19 +22,22 @@ NewPipe contribution guidelines | |||||||||||
|
|
||||||||||||
| ## Crash reporting | ||||||||||||
|
|
||||||||||||
| Report crashes through the **automated crash report system** of NewPipe. | ||||||||||||
| This way all the data needed for debugging is included in your bug report for GitHub. | ||||||||||||
| Report crashes through the **automated crash report system** of NewPipe. | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to briefly explain how the "automated crash report system" works or how to go about it. It might not be obvious to a new contributor |
||||||||||||
| This way all the data needed for debugging is included in your bug report for GitHub. | ||||||||||||
| You'll see *exactly* what is sent, be able to add **your comments**, and then send it. | ||||||||||||
|
|
||||||||||||
| ## Issue reporting/feature requests | ||||||||||||
|
|
||||||||||||
| * **Already reported**? Browse the [existing issues](https://github.com/TeamNewPipe/NewPipe/issues) to make sure your issue/feature hasn't been reported/requested. | ||||||||||||
| * **Already fixed**? Check whether your issue/feature is already fixed/implemented. | ||||||||||||
| * **Still relevant**? Check if the issue still exists in the latest release/beta version. | ||||||||||||
| * **Already reported**? Please search through [existing issues](https://github.com/TeamNewPipe/NewPipe/issues) (both [open **and** closed](https://github.com/TeamNewPipe/NewPipe/issues?q=is%3Aissue)) to make sure your issue isn't already reported. Duplicate issues will be closed. | ||||||||||||
| * **Already fixed**? Check whether your issue/feature is already fixed/implemented in NewPipe. | ||||||||||||
| * **Still relevant**? Check if the issue still exists in the latest version. | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| * **Can you fix it**? If you are an Android/Java developer, you are always welcome to fix an issue or implement a feature yourself. PRs welcome! See [Code contribution](#code-contribution) for more info. | ||||||||||||
| * **Is it in English**? Issues in other languages will be ignored unless someone translates them. | ||||||||||||
| * This does not supercede the notice at the top: **only bugfix and refactor PRs will be accepted unless explicitly permitted** | ||||||||||||
| * **Is it in English**? Please do not open issues in other languages. Non-English issues will be ignored or closed unless translated | ||||||||||||
| * **Is it one issue**? Multiple issues require multiple reports, that can be linked to track their statuses. | ||||||||||||
| * ⚠ **Multiple issues in one issue will be closed and you will be advised to open them as individual issues** | ||||||||||||
| * **The template**: Fill it out, everyone wins. Your issue has a chance of getting fixed. | ||||||||||||
| * ⚠ **Failure to fill in the template may result in your issue being closed without warning.** | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| ## Translation | ||||||||||||
|
|
@@ -39,35 +46,124 @@ You'll see *exactly* what is sent, be able to add **your comments**, and then se | |||||||||||
| * Add the language you want to translate if it is not there already: see [How to add a new language](https://github.com/TeamNewPipe/NewPipe/wiki/How-to-add-a-new-language-to-NewPipe) in the wiki. | ||||||||||||
| * NewPipe uses the [PrettyTime](https://github.com/ocpsoft/prettytime) library to display localized versions of dates and times. It needs to be translated, too. Read [these instructions to add a new language](https://www.ocpsoft.org/prettytime/#section-14) and [this issue](https://github.com/TeamNewPipe/NewPipe/issues/9134) for more info. | ||||||||||||
|
|
||||||||||||
| ## Code contribution | ||||||||||||
| # Code contribution | ||||||||||||
|
|
||||||||||||
| ### Guidelines | ||||||||||||
| ## Guidelines | ||||||||||||
|
|
||||||||||||
| * Stick to NewPipe's *style conventions* of [checkStyle](https://github.com/checkstyle/checkstyle) and [ktlint](https://github.com/pinterest/ktlint). They run each time you build the project. | ||||||||||||
| * Stick to [F-Droid contribution guidelines](https://f-droid.org/wiki/page/Inclusion_Policy). | ||||||||||||
| * In particular **do not bring non-free software** (e.g. binary blobs) into the project. Make sure you do not introduce any closed-source library from Google. | ||||||||||||
|
|
||||||||||||
| ### Before starting development | ||||||||||||
| ## Before starting development | ||||||||||||
|
|
||||||||||||
| ### Declare intent | ||||||||||||
|
|
||||||||||||
| If you want to help out with an existing bug report or feature request, **leave a comment** on that issue saying you want to work on it and wait until you are assigned. **We do not welcome unsolicited PRs**. | ||||||||||||
|
|
||||||||||||
| ❗ **YOU MUST** explain how you plan to implement the feature/fix the bug. This lets the team and community give feedback on your solution to know if it's viable or not, so you don't spend time working on something that should be done differently, should be avoided completely, or is otherwise incorrect. | ||||||||||||
|
|
||||||||||||
| ------ | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| * If there is no existing issue for what you want to work on, **open a new one**, and give a comprehensive explanation of the feature/bug and how you plan to implement your solution, as per what is written above | ||||||||||||
| * Please show **intent to maintain your features** and code after you contribute a PR. Unmaintained code is a hassle for core developers. If you do not intend to maintain features you plan to contribute, please rethink your submission, or clearly state that in the PR description. | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| ## Creating a Pull Request (PR) | ||||||||||||
|
|
||||||||||||
| See [Forking a repo](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo) and [Creating a PR from a fork](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) for instructions on how to make a PR. | ||||||||||||
|
|
||||||||||||
| Fork the repository, and make your changes on a **separate branch** with a meaningful name, *not* on `master` or `dev`. | ||||||||||||
|
|
||||||||||||
| **YOU MUST** fill in the PR template. Failure to do so will mean your PR will not be reviewed and may be closed. | ||||||||||||
|
|
||||||||||||
| ### Small and focused PRs | ||||||||||||
|
|
||||||||||||
| So that PRs can be reviewed quickly and easily, **YOU MUST** ensure your PR targets **only one specific issue/solution/bug**. | ||||||||||||
| PRs must be focused and kept as small as possible to make them quicker to review, and means they can be reviewed and merged in parallel, instead of one PR containing 5 things that can all blocked from merging because of just 1 of them. | ||||||||||||
|
|
||||||||||||
| * PRs that just so happen to solve multiple issues and cannot be feasiibly reduced further are an exception | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
|
||||||||||||
| <details><summary><b>Useful info on making reviewable PRs</b></summary> | ||||||||||||
| <ul> | ||||||||||||
| <li>https://engineering.joinknack.com/art-and-science-of-reviewable-prs/</li> | ||||||||||||
| <li>https://zulip.readthedocs.io/en/latest/contributing/reviewable-prs.html</li> | ||||||||||||
| <li>https://artsy.github.io/blog/2021/03/09/strategies-for-small-focused-pull-requests/</li> | ||||||||||||
| <li>https://fosdem.org/2026/schedule/event/L7ERNP-prs-maintainers-will-love/</li> | ||||||||||||
| <li>https://graphite.com/guides/best-practices-managing-pr-size</li> | ||||||||||||
| <li>https://www.propelcode.ai/blog/pr-size-impact-code-review-quality-data-study</li> | ||||||||||||
| <li>https://bssw.io/blog_posts/pull-request-size-matters</li> | ||||||||||||
| <li>https://scicomp.aalto.fi/scicomp/practical-git-prs/</li> | ||||||||||||
| </ul> | ||||||||||||
| </details> | ||||||||||||
|
|
||||||||||||
| **Caveat:** our codebase is very old so you will absolutely see things that can easily be refactored as you browse through files. | ||||||||||||
| Feel free to make small refactors alongside your changes, but ensure they don't clog the PR or add too much cognitive load. | ||||||||||||
|
|
||||||||||||
| ### Splitting work over multiple PRs | ||||||||||||
|
|
||||||||||||
| PRs that cannot fully address an issue in one PR without being too big must: | ||||||||||||
| * Be split up into multiple PRs | ||||||||||||
| * and, if applicable, the issue the PR addresses must also be split up into multiple sub-issues (if it can be logically separated into different issues that aren't just Issue 1, Issue 2, etc. Although this may imply the issue breaks the one issue for one thing rule) | ||||||||||||
|
|
||||||||||||
| ### Average PR review time | ||||||||||||
|
|
||||||||||||
| PRs should be reviewable within **2-3 dev hrs on average**, and a maxiumum of 6 dev hours. | ||||||||||||
|
|
||||||||||||
| 1 dev hour = 1 hour focused entirely on reviewing the PR without breaks or distractions. | ||||||||||||
|
|
||||||||||||
| If it would take someone more than 3 hours to fully understand your PR to the point as if they had the written it themselves, then your PR is too big. | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
|
||||||||||||
| * We reserve the right to reject PRs that are too big and request they be split up. | ||||||||||||
|
|
||||||||||||
| ### All changes must be tested | ||||||||||||
|
|
||||||||||||
| ❗ Before opening a PR **YOU MUST** test (compile and run) your changes and ensure they work. Detail precisely what testing has been done in the PR description so that reviewers are easily able to replicate and verify. | ||||||||||||
|
|
||||||||||||
| NewPipe uses [NewPipeExtractor](https://github.com/TeamNewPipe/NewPipeExtractor) to fetch data from services. If you need to change something there, you must test your changes in NewPipe. | ||||||||||||
| * See the comments in [libs.versions.toml](https://github.com/TeamNewPipe/NewPipe/blob/dev/gradle/libs.versions.toml#L72) on how to use an extractor build from github | ||||||||||||
| * See the comment at the bottom of [settings.gradle.kts](https://github.com/TeamNewPipe/NewPipe/blob/dev/settings.gradle.kts) on how to use your own extractor locally | ||||||||||||
| <!-- **TODO:** Update [extractor documentation](https://github.com/TeamNewPipe/documentation) to be accurate and replace with this | ||||||||||||
| See [here](https://teamnewpipe.github.io/documentation/04_Run_changes_in_App/) for instructions on running NewPipe with your own extractor. --> | ||||||||||||
|
|
||||||||||||
| * **All NewPipeExtractor code must be tested with unit/integration tests** | ||||||||||||
| * All bug fixes must include a regression test of said bug that verifies it works with the fix and fails without the fix | ||||||||||||
|
|
||||||||||||
| <!-- TODO: uncomment when we have appropriate testing infrastructure | ||||||||||||
| * **All NewPipe app code must be tested with unit/integration/UI tests** | ||||||||||||
| * All bug fixes must include a regression test of said bug that verifies it works with the fix and fails without the fix | ||||||||||||
| * Whatever steps must be taken manually in the app to test the changes in your PR must be encapsulated in an automated test | ||||||||||||
| * Likewise any UI changes must be include associated UI tests --> | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| ### Please explain your PR changes | ||||||||||||
|
|
||||||||||||
| ❗ **YOU MUST** include an in-depth comprehensive explanation of your PR changes in the PR description, to save the time of people reviewing your PR | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Alternative Important YOU MUST include an in-depth comprehensive explanation of your PR changes in the PR description, to save the time of people reviewing your PR.
Suggested change
|
||||||||||||
|
|
||||||||||||
| This explanation must be easy to understand so people don't spend hours looking at your code to understand what it does, why it does what it does, or what makes it work. | ||||||||||||
|
|
||||||||||||
| This will both serve as documentation and time-saving. Keep in mind not everyone knows every part of the codebase or every part of Android, so please make your explanation as straightforward as possible to understand for other _Android developers_. That is, assume readers have good technical expertise but just do not understand that part of the codebase. | ||||||||||||
|
|
||||||||||||
| The expectation is that reviewers **should not** have to read the code in-depth to understand _how_ the PR implements a feature or fixes a bug. The PR description should do most of the heavy lifting of explaining the code changes. If you keep your PR small and focused this should not be that hard to do. | ||||||||||||
|
|
||||||||||||
| This will make things easier for everyone and enable your PR to be reviewed by most of the team and not a select few who are already familiar with that area of the codebase, and PRs can be reviewed and merged within 1-2 days instead of being open for weeks and months :). | ||||||||||||
|
|
||||||||||||
| * If you want to help out with an existing bug report or feature request, **leave a comment** on that issue saying you want to try your hand at it. | ||||||||||||
| * If there is no existing issue for what you want to work on, **open a new one** describing the changes you are planning to introduce. This gives the team and the community a chance to give **feedback** before you spend time on something that is already in development, should be done differently, or should be avoided completely. | ||||||||||||
| * Please show **intention to maintain your features** and code after you contribute a PR. Unmaintained code is a hassle for core developers. If you do not intend to maintain features you plan to contribute, please rethink your submission, or clearly state that in the PR description. | ||||||||||||
| * Create PRs that cover only **one specific issue/solution/bug**. Do not create PRs that are huge monoliths and could have been split into multiple independent contributions. | ||||||||||||
| * NewPipe uses [NewPipeExtractor](https://github.com/TeamNewPipe/NewPipeExtractor) to fetch data from services. If you need to change something there, you must test your changes in NewPipe. Telling NewPipe to use your extractor version can be accomplished by editing the `app/build.gradle` file: the comments under the "NewPipe libraries" section of `dependencies` will help you out. | ||||||||||||
| ## Other requirements | ||||||||||||
|
|
||||||||||||
| ### Creating a Pull Request (PR) | ||||||||||||
| * Any PR that changes any part of the UI **must** include screenshots/recordings of **ALL** affected UI screens/components/workflows. Reviewers should not have to manually run your changes to see what the UI changes look like: they should be documented in the PR. | ||||||||||||
| * Respond if team members request changes or otherwise raise issues about your PRs. | ||||||||||||
| * You are not obligated to do so for things addressed by non-team members (anyone can say anything on PRs); however you are still advised to do so as most of the time their points will be relevant | ||||||||||||
| * It is **your** responsibility to ensure CI builds/tests pass on your PR | ||||||||||||
| * The only exception is when the failure has nothing to do with your PR | ||||||||||||
| * Please ensure your PR is **up-to-date** with the rest of the code and resolve conflicts as and when they appear. | ||||||||||||
| * We like to keep clean commit history: so you may update your PR via merging or rebasing as you see fit, but once it's ready to be merged **you must ensure your PR has linear history.** | ||||||||||||
|
|
||||||||||||
| * Make changes on a **separate branch** with a meaningful name, not on the _master_ branch or the _dev_ branch. This is commonly known as *feature branch workflow*. You may then send your changes as a pull request (PR) on GitHub against the `dev` branch. | ||||||||||||
| * Please **test** (compile and run) your code before submitting changes! Ideally, provide test feedback in the PR description. Untested code will **not** be merged! | ||||||||||||
| * Respond if someone requests changes or otherwise raises issues about your PRs. | ||||||||||||
| * Try to figure out yourself why builds on our CI fail. | ||||||||||||
| * Make sure your PR is **up-to-date** with the rest of the code. Often, a simple click on "Update branch" will do the job, but if not, you must *rebase* your branch on the `dev` branch manually and resolve the conflicts on your own. You can find help [on the wiki](https://github.com/TeamNewPipe/NewPipe/wiki/How-to-merge-a-PR). Doing this makes the maintainers' job way easier. | ||||||||||||
|
|
||||||||||||
| ## IDE setup & building the app | ||||||||||||
|
|
||||||||||||
| ### Basic setup | ||||||||||||
|
|
||||||||||||
| NewPipe is developed using [Android Studio](https://developer.android.com/studio/). Learn more about how to install it and how it works in the [official documentation](https://developer.android.com/studio/intro). In particular, make sure you have accepted Android Studio's SDK licences. Once Android Studio is ready, setting up the NewPipe project is fairly simple: | ||||||||||||
| NewPipe is developed using [Android Studio](https://developer.android.com/studio/). See the [official documentation](https://developer.android.com/studio/intro) for a short brief on how it works. Once installed, setting up NewPipe is fairly simple: | ||||||||||||
| - Clone the NewPipe repository with `git clone https://github.com/TeamNewPipe/NewPipe.git` (or use the link from your own fork, if you want to open a PR). | ||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will look better if the
|
||||||||||||
| - Open the folder you just cloned with Android Studio. | ||||||||||||
| - Build and run it just like you would do with any other app, with the green triangle in the top bar. | ||||||||||||
|
Comment on lines
168
to
169
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not that relevant but would be nice to mention this being a compose multiplatform project and any scenario of contributing to or building the iOS version having XCode installed should be a must-have.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not well versed with how that works: is there an official docs I can link to that has this info, instead of me trying to figure out the best way to say it?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "Set up the environment" section of the docs here has the info. Just to add Android Studio or IntelliJ will be ideal for the project not just Android Studio |
||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.