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

Dealing with new examples or code changes #9

Open
sclausen opened this issue Jan 5, 2016 · 8 comments
Open

Dealing with new examples or code changes #9

sclausen opened this issue Jan 5, 2016 · 8 comments
Labels

Comments

@sclausen
Copy link
Contributor

sclausen commented Jan 5, 2016

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
Copy link
Contributor

mgechev commented Jan 6, 2016

Using pull requests sounds the best option to me.

// cc @NathanWalker @ludohenin @briantopping @Bigous

@sclausen
Copy link
Contributor Author

sclausen commented Jan 6, 2016

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
Copy link

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
Copy link

@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
Copy link
Contributor Author

sclausen commented Jan 6, 2016

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

@Bigous
Copy link

Bigous commented Jan 6, 2016

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
Copy link

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
Copy link
Contributor

I agree with @Bigous here as well 👍

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

No branches or pull requests

5 participants