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 to contribution guidelines #124

Merged
merged 11 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 55 additions & 50 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,33 @@
# Contributing

Contributions are always welcome, no matter how large or small!
Thank you for considering contributing to the Klaviyo SDK!
evan-masseau marked this conversation as resolved.
Show resolved Hide resolved

We want this community to be friendly and respectful to each other. Please follow it in all your interactions with the project. Before contributing, please read the [code of conduct](./CODE_OF_CONDUCT.md).
We welcome your contributions and strive to respond in a timely manner. In return, we ask that you do your
**due diligence** to answer your own questions using public resources, and check for related issues (including
closed ones) before posting. This helps keep the discussion focused on the most important topics. Issues deemed
off-topic or out of scope for the SDK will be closed.

Before contributing, please read the [code of conduct](./CODE_OF_CONDUCT.md). We want this community to be friendly
and respectful to each other. Please follow it in all your interactions with the project.

## Github Issues

If you suspect a bug or have a feature request, please open an issue, following the guidelines below:
evan-masseau marked this conversation as resolved.
Show resolved Hide resolved

- Research your issue using public resources such as Google, Stack Overflow, React Native documentation, etc.
- Attempt to reproduce your issue with the example app provided in the repository. Setup instruction can be found below.
- Check if the issue has already been reported before.
- Use a clear and descriptive title for the issue to identify the problem.
- Include as much information as possible, including:
- The version of the SDK you are using.
- The version of React Native you are using.
- The platform (iOS or Android) you are experiencing the issue on.
- Any error messages you are seeing.
- The expected behavior and what went wrong.
- Detailed steps to reproduce the issue
- A code snippet or a minimal example that reproduces the issue.

> ⚠️ Answer all questions in the issue template. Incomplete issues will be de-prioritized or closed. ⚠️

## Development workflow

Expand All @@ -11,19 +36,29 @@ This project is a monorepo managed using [Yarn workspaces](https://yarnpkg.com/f
- The library package in the root directory.
- An example app in the `example/` directory.

To get started with the project, run `yarn` in the root directory to install the required dependencies for each package:
To get started with the project, run the following in the root directory to install the required dependencies for each package:

```sh
yarn example-setup
```

And configure the pre-commit hooks with:

```sh
yarn
yarn husky install
```

> Since the project relies on Yarn workspaces, you cannot use [`npm`](https://github.com/npm/cli) for development.

The [example app](/example/) demonstrates usage of the library. You need to run it to test any changes you make.
The [example app](/example) demonstrates usage of the library. You need to run it to test any changes you make.

It is configured to use the local version of the library, so any changes you make to the library's source code will be reflected in the example app. Changes to the library's JavaScript code will be reflected in the example app without a rebuild, but native code changes will require a rebuild of the example app.
It is configured to use the local version of the library, so any changes you make to the library's source code will be
reflected in the example app. Changes to the library's JavaScript code will be reflected in the example app without a
rebuild, but native code changes will require a rebuild of the example app.

If you want to use Android Studio or XCode to edit the native code, you can open the `example/android` or `example/ios` directories respectively in those editors. To edit the Objective-C or Swift files, open `example/ios/KlaviyoReactNativeSdkExample.xcworkspace` in XCode and find the source files at `Pods > Development Pods > klaviyo-react-native-sdk`.
If you want to use Android Studio or XCode to edit the native code, you can open the `example/android` or `example/ios`
directories respectively in those editors. To edit the Objective-C or Swift files, open `example/ios/KlaviyoReactNativeSdkExample.xcworkspace`
in XCode and find the source files at `Pods > Development Pods > klaviyo-react-native-sdk`.

To edit the Java or Kotlin files, open `example/android` in Android studio and find the source files at `klaviyo-react-native-sdk` under `Android`.
evan-masseau marked this conversation as resolved.
Show resolved Hide resolved

Expand All @@ -47,35 +82,6 @@ To run the example app on iOS:
yarn example ios
```

By default, the example is configured to build with the old architecture. To run the example with the new architecture, you can do the following:
evan-masseau marked this conversation as resolved.
Show resolved Hide resolved

1. For Android, run:

```sh
ORG_GRADLE_PROJECT_newArchEnabled=true yarn example android
```

2. For iOS, run:

```sh
RCT_NEW_ARCH_ENABLED=1 yarn pod-install example/ios
yarn example ios
```

If you are building for a different architecture than your previous build, make sure to remove the build folders first. You can run the following command to cleanup all build folders:

```sh
yarn clean
```

To confirm that the app is running with the new architecture, you can check the Metro logs for a message like this:

```sh
Running "KlaviyoReactNativeSdkExample" with {"fabric":true,"initialProps":{"concurrentRoot":true},"rootTag":1}
```

Note the `"fabric":true` and `"concurrentRoot":true` properties.

Make sure your code passes TypeScript and ESLint. Run the following to verify:

```sh
Expand All @@ -86,7 +92,7 @@ yarn lint
To fix formatting errors, run the following:

```sh
yarn lint --fix
yarn lint-fix
```

Remember to add tests for your change if possible. Run the unit tests by:
Expand All @@ -106,31 +112,30 @@ We follow the [conventional commits specification](https://www.conventionalcommi
- `test`: adding or updating tests, e.g. add integration tests using detox.
- `chore`: tooling changes, e.g. change CI config.

Our pre-commit hooks verify that your commit message matches this format when committing.

### Linting and tests

[ESLint](https://eslint.org/), [Prettier](https://prettier.io/), [TypeScript](https://www.typescriptlang.org/)
For pre-commit file formatters/linters, you may need to install the following if you don’t have them already:

We use [TypeScript](https://www.typescriptlang.org/) for type checking, [ESLint](https://eslint.org/) with [Prettier](https://prettier.io/) for linting and formatting the code, and [Jest](https://jestjs.io/) for testing.

Our pre-commit hooks verify that the linter and tests pass when committing.

### Publishing to npm
```sh
which ktlint
# if not found, install with homebrew:
brew install ktlint

We use [release-it](https://github.com/release-it/release-it) to make it easier to publish new versions. It handles common tasks like bumping version based on semver, creating tags and releases etc.
which swiftlint
# if not found install with homebrew
brew install swiftlint
```

To publish new versions, run the following:
We use [TypeScript](https://www.typescriptlang.org/) for type checking, [ESLint](https://eslint.org/), SwiftLint and KtLint with [Prettier](https://prettier.io/) for linting and formatting the code, and [Jest](https://jestjs.io/) for testing.

```sh
yarn release
```
Our pre-commit hooks verify that the linter and tests pass when committing.

### Scripts

The `package.json` file contains various scripts for common tasks:

- `yarn`: setup project by installing dependencies.
- `yarn example-setup`: install dependencies for the example app
- `yarn typecheck`: type-check files with TypeScript.
- `yarn lint`: lint files with ESLint.
- `yarn test`: run unit tests with Jest.
Expand Down
25 changes: 17 additions & 8 deletions .github/ISSUE_TEMPLATE/bug.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,31 @@ body:
value: |
Thank you for contributing to the Klaviyo React Native SDK!

Before you submit your issue, please complete each text area below with the relevant details for your bug, and complete the steps in the checklist.
Before you submit your issue, please read our [contributing guidelines](./CONTRIBUTING.md) and complete each
text area below with the relevant details for your bug.

We welcome your input! If you have any code suggestions regarding your issue, feel free to [submit a pull request](https://github.com/klaviyo/klaviyo-react-native-sdk/pulls) after creating an issue.
Note: incomplete issues may be de-prioritized or closed.

We welcome your input! If you have any code suggestions regarding your issue, feel free to
[submit a pull request](https://github.com/klaviyo/klaviyo-react-native-sdk/pulls) after creating an issue.
- type: textarea
attributes:
label: Description
render: markdown
description: |
A short description of the incorrect behavior.

If you think this issue has been recently introduced and did not occur in an earlier version, please note that. If possible, include the last version that the behavior was correct in addition to your current version.
If you think this issue has been recently introduced and did not occur in an earlier version, please note that.
If possible, include the last version that the behavior was correct in addition to your current version.
validations:
required: true
- type: checkboxes
attributes:
label: Checklist
options:
- label: I have determined whether this bug is also reproducible in a vanilla project
- label: I have determined whether this bug is also reproducible in a vanilla project, such as the example app in this repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be overkill to add a checkbox for, i've read and understand the contributing policy or something to that effect? can be added to bugs and feature templates

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea

required: false
- label: If possible, I've reproduced the issue using the `master` branch of this package.
- label: If possible, I've reproduced the issue using the `master` branch or latest release of this package.
required: false
- label: This issue hasn't been addressed in an [existing GitHub issue](https://github.com/klaviyo/klaviyo-react-native-sdk/issues) or [pull request](https://github.com/klaviyo/klaviyo-react-native-sdk/pulls).
required: true
Expand All @@ -36,14 +41,14 @@ body:
render: markdown
description: Describe what you expected to happen.
validations:
required: false
required: true
- type: textarea
attributes:
label: Actual behavior
render: markdown
description: Describe or copy/paste the behavior you observe. Include as much detail as possible, including stack traces or screenshots if available.
validations:
required: false
required: true
- type: textarea
attributes:
label: Steps to reproduce
Expand All @@ -55,14 +60,18 @@ body:
placeholder: |
1. ...
validations:
required: false
required: true
- type: input
attributes:
label: The Klaviyo React Native SDK version information
description: The version of the Klaviyo React Native SDK used to reproduce this issue.
placeholder: "'0.1.0' for example, or a commit hash"
validations:
required: true
- type: input
attributes:
label: Environment Description
description: Include the device model, OS version, and any other relevant development environment information.
placeholder: 'Run `react-native info` in your project directory and paste the output here. Include additional information such as device model, OS version, etc.'
validations:
required: true
6 changes: 4 additions & 2 deletions .github/ISSUE_TEMPLATE/feature.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ body:
value: |
Thank you for contributing to the Klaviyo React Native SDK!

Before you submit your issue, please complete each text area below with as much detail as you can.
Before you submit your issue, please read our [contributing guidelines](./CONTRIBUTING.md) and complete each
text area below with as much detail as you can.

We welcome your input! If you have any code suggestions regarding your issue, feel free to [submit a pull request](https://github.com/klaviyo/klaviyo-react-native-sdk/pulls) after creating an issue.
We welcome your input! If you have any code suggestions regarding your issue, feel free to
[submit a pull request](https://github.com/klaviyo/klaviyo-react-native-sdk/pulls) after creating an issue.
- type: textarea
attributes:
label: Description
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ jobs:
curl -sSLO https://github.com/pinterest/ktlint/releases/download/1.1.0/ktlint && chmod a+x ktlint && sudo mv ktlint /usr/local/bin/

- name: Lint files
run: yarn lint
run: |
yarn lint
.husky/pre-commit

- name: Typecheck files
run: yarn typecheck
Expand Down
5 changes: 5 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
. "$(dirname -- "$0")/_/husky.sh"

yarn lint-staged

if [ -f "package-lock.json" ];
then echo "package-lock.json is not allowed";
exit 1;
fi
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ In order to collect the APNs push token in your React Native code you need to:
FIRMessaging.messaging.APNSToken = deviceToken;
```
5. Finally, in your React Native code, you can collect & set the push token as follows:

```typescript
import messaging from '@react-native-firebase/messaging';
import { Klaviyo } from 'klaviyo-react-native-sdk';
Expand Down Expand Up @@ -376,8 +377,10 @@ native code. Note that either of these approaches is sufficient to inform the Kl
}
};
```

2. **Native Notification Permission**:
Follow instructions from our native SDK documentation to request permission from native code:

- [Android](https://github.com/klaviyo/klaviyo-android-sdk#collecting-push-tokens)
- [iOS](https://github.com/klaviyo/klaviyo-swift-sdk?tab=readme-ov-file#request-push-notification-permission)

Expand Down Expand Up @@ -446,13 +449,14 @@ Linking.getInitialURL().then((url) => {
## Troubleshooting

Use the [troubleshooting guide](Troubleshooting.md) to resolve common issues with the Klaviyo React Native SDK.
If the issues you are facing isn't in the troubleshooting guide, and you believe it's a bug in the SDK, please file an issue in our repository.
If the issues you are facing isn't in the troubleshooting guide, and you believe it's a bug in the SDK,
please file an [issue](https://github.com/klaviyo/klaviyo-react-native-sdk/issues) in our repository.

## Contributing

Refer to the [contributing guide](.github/CONTRIBUTING.md) to learn how to contribute to the Klaviyo React Native SDK.
We welcome your feedback in the [discussion](https://github.com/klaviyo/klaviyo-react-native-sdk/discussions)
and [issues](https://github.com/klaviyo/klaviyo-react-native-sdk/issues) sections of our public GitHub repository.
We welcome your feedback, feature requests or other ideas in the
evan-masseau marked this conversation as resolved.
Show resolved Hide resolved
[issues](https://github.com/klaviyo/klaviyo-react-native-sdk/issues) section of our public GitHub repository.

## License

Expand Down
Loading
Loading