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

chore: update contributing guide #2140

Merged
merged 33 commits into from
Jun 11, 2024

Conversation

maciekstosio
Copy link
Contributor

Description

Our contributing guide was very basic and written long time ago. I updated it with some more insights and details on how to start contributing to this project.

@maciekstosio maciekstosio requested review from tboba and kkafar May 17, 2024 12:03
Copy link
Member

@tboba tboba left a comment

Choose a reason for hiding this comment

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

Great stuff! Left my review below 👇

guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
maciekstosio and others added 8 commits May 17, 2024 16:09
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

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

My initial impressions. I'll reiterate on this PR

guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved
guides/CONTRIBUTING.md Outdated Show resolved Hide resolved

For commits and pull request names we follow a [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification.
> [!WARNING]
> Those steps are crucial, if you change the API in react-native-screens and won't merge react-navigation changes the libraries may go out of sync and crash i.e. because of not existing property. On the other hand, if you don't perform step 6 and 7, test examples and showcase app may stop working in react-native-screens.
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit exaggerated. Any backward compatible changes in react-native-screens, so changes in minor / patch versions should go unnoticed if react-navigation is not updated. Only breaking changes would break the behaviour and we're fine with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So my idea was that the previous sections let you know if you need or do not need to open PR to react navigation. And if you need, you end up in this section. Then those steps are crucial (?). But I can rephrase that if it's not clear.

@kkafar kkafar self-requested a review May 20, 2024 12:36
maciekstosio and others added 14 commits May 21, 2024 09:20
Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
@maciekstosio maciekstosio requested a review from tboba May 21, 2024 11:09
@tboba
Copy link
Member

tboba commented May 22, 2024

Let's wait with merging this PR for #2135 and #2149, since there may be some changes in the contributing guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants