Skip to content

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
Contributor

@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 👇

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


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
Contributor

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.

ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
## 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.

---------

Co-authored-by: Tymoteusz Boba <Tymoteusz.Boba@gmail.com>
Co-authored-by: Kacper Kafara <kacper.kafara@swmansion.com>
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