-
Notifications
You must be signed in to change notification settings - Fork 734
Adding a guide for contributors #1433
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
b0a3c0c
Contribution guide
ethanshar a99d282
Update CONTRIBUTING.md
ethanshar 2c4653d
Update CONTRIBUTING.md
ethanshar 36682a8
Update CONTRIBUTING.md
ethanshar cf1008f
Update CONTRIBUTING.md
ethanshar ecffd6c
Update CONTRIBUTING.md
ethanshar d540611
Update CONTRIBUTING.md
ethanshar 9dc2e3a
Add a bullet on PR prefixes
ethanshar 5f58ffb
Add a secion on controlled/uncontrolled API
ethanshar ad51287
Update CONTRIBUTING.md
ethanshar 8588bb9
add another bullet to code standards
ethanshar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# Contributing | ||
If you got here it means you want to dedicate some of your time and help us improve this library. | ||
So first of all, thank you ❤️ | ||
|
||
When contributing to this repository, please first discuss the change you wish to make via [issue](https://github.com/wix/react-native-ui-lib/issues/new/choose), | ||
[Discord channel](https://discord.gg/2eW4g6Z), or any other method with the owners of this repository before making a change. | ||
|
||
## Pull Request Process | ||
|
||
1. First make sure the environment is working and synced with master. For installation details go [here](https://github.com/wix/react-native-ui-lib/blob/master/markdowns/getting-started/setup.md#demo-app) | ||
2. Before submitting a PR we suggest running `npm run prepush` command that verifies our lint, TS and tests were not broken. | ||
3. Please use our PR prefixes accordingly. `fix` for bugfix, `feat` for feature, `infra` for infrastructure changes and `docs` for documentation related changes (e.g `fix/ButtonStyle`, `feat/Avatar_colorProp`) | ||
4. Please don't change our PR template. | ||
- In the Description section add everything that can help the reviewer and the reviewing process, like a description of the issue and the way you decided to go about it, screenshots or gifs, etc. | ||
- The Changelog section is important. Write a message for the library users - this message will be included in our release notes. | ||
5. We try to make the PR review process as quick as possible, please be patient | ||
|
||
ethanshar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Code Standards | ||
|
||
- Readability and clean code above all - we believe a clean code is easier to maintain and refactor when needed. | ||
- Deciding on an API is not trivial but we always aim to keep it generic and intuitive - feel free to consult with us. | ||
- Our documentation is derived from our components' prop comments, make sure to add a clear description when adding new props. | ||
- When possible, consider adding tests for the new functionality you added. | ||
- Try separating logic from UI as much as possible using hooks and presenters. | ||
- Keep components small and single purposed. | ||
|
||
#### Controlled vs Uncontrolled components | ||
When facing a new component's API we usually aim for making it controlled. | ||
This is in order to give the user most control over what will be rendered in the component. | ||
Having said that, keep in mind that in some cases (mostly due to performance concerns) we decide on making a component uncontrolled. | ||
|
||
|
||
mendyEdri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
## Typescript Status | ||
Currently most of our code is migrated to typescript though we still have some leftovers of javascript code and manual typings. | ||
Until we complete the migration you are required to do the following for new TS files | ||
|
||
Before pushing new code make sure to run `npm run build:dev` - this check for TS errors and create appropriate declarations (`d.ts`) files under generatedTypes folder. Make sure to push these files as well! |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.