search

react-native-elements / playground

Exploring Possibilities with React Native Elements
https://react-native-elements.js.org
MIT License
19 stars 43 forks source link

feat:Adding prettier as a dev dependency and formatting the whole code #59

Closed abhishekkumar08 closed 3 years ago

abhishekkumar08 commented 3 years ago

Fixes:#58

netlify[bot] commented 3 years ago

Deploy preview for rne-playground ready!

Built with commit 724040fada71f9b44ecdf1af8fdd918f93c07cf3

https://deploy-preview-59--rne-playground.netlify.app

Uyadav207 commented 3 years ago

@abhishekkumar08, please have a look at ......

abhishekkumar08 commented 3 years ago

@abhishekkumar08, please have a look at ......

* https://reactnativeelements.com/docs/contributing

If you try to understand the PR, it adds prettier to the code base which is a code formatter, I just installed it as a dev dependency and ran the command for it to format the whole code base. Thanks for the review. @Uyadav207

Uyadav207 commented 3 years ago

@abhishekkumar08, ok if it's prettier also... the changes made are quite huge and this will create merge conflict... for the previous PR, as I have faced the same problem yesterday. 😢 and had a hard time resolving conflict.

I hope you understand? 😊

abhishekkumar08 commented 3 years ago

@Uyadav207 Yeah bro I got your point 😄 . Thanks for the suggestion, any other way how should I do this change?

tewarig commented 3 years ago

@Uyadav207 Yeah bro I got your point. Thanks for the suggestion, any other way how should I do this change?

just keep an eye on the repo if any pr is merged in this repo make sure you update your repo and format its code again

Uyadav207 commented 3 years ago

but issue #56 #57 and pr made on these are almost the same adding prettier as a dev dependency is not a good idea. we need to run npm run format every time before we make a pr?

in #56 we are adding a file... which will automatically format the code in vs code if the prettier

the extension is installed.

Exactly.

Uyadav207 commented 3 years ago

@Uyadav207 Yeah bro I got your point 😄 . Thanks for the suggestion, any other way how should I do this change?

This is a breaking change. Need to wait for all PR to get merge than if we push your change.... Then it would not cause any conflict... but syncing the fork then would be necessary for all contributors... Is this making any sense ?

jugshaurya commented 3 years ago

@abhishekkumar08 already created an issue and made a PR about the same thing earlier than you. Kindly work on some other issues.

jugshaurya commented 3 years ago

@Uyadav207 Yeah bro I got your point smile . Thanks for the suggestion, any other way how should I do this change?

This is a breaking change. Need to wait for all PR to get merge than if we push your change.... Then it would not cause any conflict... but syncing the fork then would be necessary for all contributors... Is this making any sense ?

@Uyadav207 Initially it may seem a conflicting change but it will be beneficial in long go as developers contributing to the project will be following same styling.

tewarig commented 3 years ago

@Uyadav207 Yeah bro I got your point smile . Thanks for the suggestion, any other way how should I do this change?

This is a breaking change. Need to wait for all PR to get merge than if we push your change.... Then it would not cause any conflict... but syncing the fork then would be necessary for all contributors... Is this making any sense ?

@Uyadav207 Initially it may seem a conflicting change but it will be beneficial in long go as developers contributing to the project will be following same styling.

yes it will help a lot .