codernaut / citizenconnect

Apache License 2.0
1 stars 9 forks source link

Adding gitignore file to android app folder #14

Closed wahibhaq closed 6 years ago

wahibhaq commented 6 years ago

Please don't be scared to see 172 file deletions :)

I have tried to review them twice but you should also thoroughly give a look to see if any required file not gets removed from remote. These are all files which shouldn't have been pushed to remote in the first place.

wahibhaq commented 6 years ago

@codernaut when you are accepting a PR to be merged, which option do you select from these?https://help.github.com/articles/about-pull-request-merges/

Normally, from what I have experienced and seen other companies using it, the workflow goes like the reviewer says 🚢 and the original author himself does the merge instead of the other way around. So onus to decide how to merge and squash etc is on the author of the PR. I would suggest to follow this model instead of the reviewer (like you in case) merging it.

I can see my individual commits in the history which should have been squashed. Maybe in future, I can squash myself before making it ready to be pushed.

Also, using labels is a very important feature. I can easily say which PR is in wip or done to avoid confusion. Also the ability to assign people for reviews.