Skip to content

Commit

Permalink
Remove unnecessary files & improve the contrubutors exp (Ivy-Apps#2918)
Browse files Browse the repository at this point in the history
* Remove unused `package.json` related to DangerJS

* Move `/assets` to `/docs/assets`

* Move the CI troubleshooting guide to `/docs`

* Simplify the PR template

* Improve CONTRIBUTING.md

* Fix typos & ambigious instructions in the PR template

* Update CONTRIBUTING.md

* Fix broken link to the "CI Troubleshooting" guide
  • Loading branch information
ILIYANGERMANOV authored Feb 5, 2024
1 parent d5a1232 commit fa3f80e
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 92 deletions.
54 changes: 6 additions & 48 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Pull Request (PR) Checklist
Please check if your pull request fulfills the following requirements:
- [ ] The PR is submitted to the `main` branch.
- [ ] I've read the [Contribution Guidelines](https://github.com/Ivy-Apps/ivy-wallet/blob/main/CONTRIBUTING.md) and my PR doesn't break the "Contributing Rules".
- [ ] I've read the [Contribution Guidelines](https://github.com/Ivy-Apps/ivy-wallet/blob/main/CONTRIBUTING.md) and my PR doesn't break the rules.
- [ ] I've read the [Architecture Guidelines](https://github.com/Ivy-Apps/ivy-wallet/blob/main/docs/Architecture.md).
- [ ] I confirm that I've run the code locally and everything works as expected.
- [ ] 🎬 I've attached a **screen recoding** of the changes.
Expand All @@ -13,15 +13,14 @@ Please check if your pull request fulfills the following requirements:
Tip: you can attach screenshots using a markdown table.
Before | After
---|---
--|--
image1 | image2
-->

Describe with a few bullets **what's new:**
- a
- b
- c
- d

> 💡 Tip: Please, attach screenshots and screen recordings. It helps a lot!
Expand All @@ -31,63 +30,22 @@ Describe with a few bullets **what's new:**

- a
- b
- c

**In what cases your code won't work?**

- a
- b
- c


## Does this PR closes any GitHub Issues?

Check **[Ivy Wallet Issues](https://github.com/Ivy-Apps/ivy-wallet/issues)**.

- Closes #ISSUE_NUMBER

> Replace `ISSUE_NUMBER` with your issue number (for example Closes #1234). If you've done that correctly, you'll see the issue title linked when previewing your PR description.
## Troubleshooting CI failures

If you see any of the PR checks failing (❌) go to [Actions](https://github.com/Ivy-Apps/ivy-wallet/actions) and find it there. Or simply click "Details" next to the failed check and explore the logs to see why it has failed.

### Detekt
[Detekt](https://detekt.dev/) is a static code analyzer for Kotlin that we use to enforce code readibility and good practices.

**To run Detekt locally:**
```
./gradlew detekt
```

If the Detekt errors are caused by a legacy code, you can suppress them using a basline.

**Detekt baseline** (not recommended)
```
./scripts/detektBaseline.sh
```

### Lint

We use the [standard Android Lint](https://developer.android.com/studio/write/lint) plus [Slack's compose-lints](https://slackhq.github.io/compose-lints/) as an addition to enforce proper Compose usage.

**To run Lint locally:**
```
./scripts/lint.sh
```

If the Lint errors are caused by a legacy code, you can suppress them using a basline.

**Lint baseline** (not recommended)
```
./scripts/lintBaseline.sh
```
> Replace `ISSUE_NUMBER` with the number of the GitHub issue that this PR is fixing. If you've done that correctly, you'll see the issue title linked when previewing your PR description.
### Unit tests
> **!Note:** Do **not** link the PR number. Link the number/id of the GitHub Issue from [issues](https://github.com/Ivy-Apps/ivy-wallet/issues).
If this job is failing this means that your changes break an existing unit test. You must identify the failing tests and fix your code.
## Troubleshooting CI failures ❌

**To run the Unit tests locally:**
```
./gradlew testDebugUnitTest
```
GitHub Actions failing? Read our [CI Troubleshooting guide](https://github.com/Ivy-Apps/ivy-wallet/blob/main/docs/CI-Troubleshooting.md).
16 changes: 8 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ What do you want to work on? [Choose an issue](https://github.com/Ivy-Apps/ivy-w

1. Browse **[Ivy Wallet Issues](https://github.com/Ivy-Apps/ivy-wallet/issues)**.
2. Choose an issue that you understand and like.
3. Comment **`I'm on it`** on the issue to let other contributors know that you're working on it.
3. Comment **`I'm on it`** on the issue so GitHub Actions can automatically assign it to you.

> ⚠️ Comment exactly `I'm on it` to take an assign, otherwise the automation for assigning issues won't work.
> ⚠️ Comment exactly **`"I'm on it"`** to take an issue, otherwise the CI automation won't work.
### Contributing Rules:

1. Comment `I'm on it` to take an issue.
2. Want to work on not in issues? Just **[create a new issue](https://github.com/Ivy-Apps/ivy-wallet/issues/new/choose)**.
3. Do **not** work on assigned issues. Ask the assignee first.
1. Comment **"I'm on it"** to take an issue.
2. Want to work on something else? Just **[create a new issue](https://github.com/Ivy-Apps/ivy-wallet/issues/new/choose)**.
3. Do **not** work on already assigned issues. Ask the assignee first.
4. Only one issue per contributor at a time.

> **Reminder:** If you take an issue, you're blocking others from working on it. Finish it fast or unassign yourself.
Expand Down Expand Up @@ -61,8 +61,8 @@ Make sure to read the **[Architecture Guidelines 🏗️](docs/Architecture.md)*
**❓ Ask yourself:**

- "Is that the simplest solution?"
- "Can I do it with less code and changes?
- "Does it work in all cases?
- "Can I do it with less code and changes?"
- "Does it work in all cases?"

## 4. Submit a PR to `main` branch

Expand All @@ -83,4 +83,4 @@ official [Ivy Wallet repo.](https://github.com/Ivy-Apps/ivy-wallet/pulls)

Ask them in [our private Telegram community](https://t.me/+ETavgioAvWg4NThk).

[![Telegram Group](https://img.shields.io/badge/Telegram-2CA5E0?style=for-the-badge&logo=telegram&logoColor=white)](https://t.me/+ETavgioAvWg4NThk)
[![Telegram Group](https://img.shields.io/badge/Telegram-2CA5E0?style=for-the-badge&logo=telegram&logoColor=white)](https://t.me/+ETavgioAvWg4NThk)
6 changes: 3 additions & 3 deletions docs/Architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Separation, the path of wisdom it is: **Compose UI ≠ Logic of Screen**. Why? H
4. **Compose Previews:** Mock `UiState`, easy it becomes. Any screen state, preview you can.
5. **Testability:** ViewModel, easy to test it is - send `UiEvent`s, verify `UiState`. Simple. Hmmm... Compose UI, like celebrity is. Mocked `UiState` if you pass, [Paparazzi](https://github.com/cashapp/paparazzi) screenshot tests pass will.

![screen-viewmodel](../assets/screen-vm.svg)
![screen-viewmodel](assets/screen-vm.svg)

### Quick understanding, you seek?

Expand All @@ -48,7 +48,7 @@ Reason, straightforward it is. More strength and simplicity, the Compose runtime

Follow [offical Guide to app architecture by Google](https://developer.android.com/topic/architecture), we do. Not for fleeting trends or mere style, but for wisdom it holds. Simplicity, at its core.

![app-architecture](../assets/app-layers.svg)
![app-architecture](assets/app-layers.svg)

### [Data Layer](https://developer.android.com/topic/architecture/data-layer)

Expand All @@ -72,7 +72,7 @@ Split our app into many modules, we do. Tangled webs ([spaghetti code](https://e

Each screen, like its own planet it is. Expanding it might, with tales of code and tales. But in its own orbit, it stays. The galaxy of code, peaceful and unshaken it remains.

![modularization-strategy](../assets/modularization.svg)
![modularization-strategy](assets/modularization.svg)

**Simple, our modularization path is:**

Expand Down
57 changes: 57 additions & 0 deletions docs/CI-Troubleshooting.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Troubleshooting CI failures

If you see any of the PR checks failing (❌) go to [Actions](https://github.com/Ivy-Apps/ivy-wallet/actions) and find it there. Or simply click "Details" next to the failed check and explore the logs to see why it has failed.

## Detekt
[Detekt](https://detekt.dev/) is a static code analyzer for Kotlin that we use to enforce code readibility and good practices.

**To run Detekt locally:**
```
./gradlew detekt
```

If the Detekt errors are caused by a legacy code, you can suppress them using a basline.

**Suppress Detekt** (only if you're sure that Detekt is wrong)

Add `@Suppress("ID_OF_THE_CHECK")` to ignore the error on a single place. For example:
```kotlin
@Suppress("FunctionMaxLength")
fun veryVeryLongFunction() {
// ...
}
```

**Detekt baseline** (not recommended)
```
./scripts/detektBaseline.sh
```

## Lint

We use the [standard Android Lint](https://developer.android.com/studio/write/lint) plus [Slack's compose-lints](https://slackhq.github.io/compose-lints/) as an addition to enforce proper Compose usage.

**To run Lint locally:**
```
./scripts/lint.sh
```

If the Lint errors are caused by a legacy code, you can suppress them using a basline.

**Suppress Lint** (only if you're sure that Lint is wrong)

Same as suppressing Detekt, just add `@Suppress("ID_OF_CHECK")`.

**Lint baseline** (not recommended)
```
./scripts/lintBaseline.sh
```

## Unit tests

If this job is failing this means that your changes break an existing unit test. You must identify the failing tests and fix your code.

**To run the Unit tests locally:**
```
./gradlew testDebugUnitTest
```
File renamed without changes.
File renamed without changes
File renamed without changes.
File renamed without changes
File renamed without changes.
File renamed without changes
25 changes: 0 additions & 25 deletions package-lock.json

This file was deleted.

8 changes: 0 additions & 8 deletions package.json

This file was deleted.

0 comments on commit fa3f80e

Please sign in to comment.