search

AngularShowcase / angular2-sample-app

Sample application built with Angular 2
MIT License
114 stars 132 forks source link

Dealing with new examples or code changes #9

Open sclausen opened 8 years ago

sclausen commented 8 years ago

I want to discuss, how we treat new stuff here.

For bigger changes, we should open issues first, but for smaller simple changes/additions, do we simply push to master, or do we create pull requests, discuss them and the proposer can, If he is member of AngularShowcase, merge it himself?

mgechev commented 8 years ago

Using pull requests sounds the best option to me.

// cc @NathanWalker @ludohenin @briantopping @Bigous

sclausen commented 8 years ago

Maybe it's a bit selfish, but I have also in mind, if one merges a pull request for someone, it appears as his contribution under "graph", not the one who actually contributed. So if I would suggest, that contributers who are member of AngularShowcase merge their pull requests self.

briantopping commented 8 years ago

I am a believer in PRs. Many projects I work on require two people from the core team, one who accepts the PR and reports it as "OK TO MERGE", a second who actually helps out and does the final merge. Either core team member could have merged, the extra set of eyes increases quality in the long term.

briantopping commented 8 years ago

@sclausen: I think you must have experience with someone not apply PRs correctly. The original author always retains credit unless there is some change to the PR by the person that does the merge.

sclausen commented 8 years ago

@briantopping could this occur, if there are two commits by different users in a PR?

Bigous commented 8 years ago

If the PR needs changes, comment at PR and let the author to change. The reviewer could just merge or give an OK to merge... The first push to a new repository is the only exception... Is there a way to someone review? The only problem with PR I had was in oracle/node-oracledb#227 ... but the merge was done by oracle using copy and paste of code, not with git merge. When they used the merge from git it was ok oracle/node-oracledb#43.

briantopping commented 8 years ago

I think @Bigous has the right path here to make sure everyone gets proper credit as well that the original author learns repository standards for future commits.

NathanWalker commented 8 years ago

I agree with @Bigous here as well :+1: