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

Documenting our new code review process #9

Closed
itsrachelfish opened this issue Feb 29, 2016 · 4 comments
Closed

Documenting our new code review process #9

itsrachelfish opened this issue Feb 29, 2016 · 4 comments

Comments

@itsrachelfish
Copy link
Member

@allanon and I have been discussing changes we'd like to see in regards to the way that code is submitted into the project. When we were using SVN, anyone with commit access would write code and push it up whenever they felt like it. There was rarely any communication between developers before changes were made or even testing done on different platforms / servers.

We'd like to make the entire development process more reliable and transparent by utilizing pull requests here on GitHub. So far we've already started using it amongst ourselves, but official documentation should be written to replace the old SVN guides like these:

http://wiki.openkore.com/index.php/Patches_Welcome
http://wiki.openkore.com/index.php/Development_Conventions

tldr; Don't push to master. Make a pull request and have it approved by another developer first.

@windhamwong
Copy link
Contributor

I love the idea but pushing to branch may affect the updates for servers.
The reviewing process is good only when all of our developers are active however we all know that we are so busy on other things and rarely updating or reviewing the codes and the system.

@itsrachelfish
Copy link
Member Author

@windhamwong I have added you as a developer to the project, however I'd still ask that you use the same process as everyone else and submit pull requests to be approved by another developer.

I understand that twRO might not have very many active developers to test, but someone else should at least read your changes before they're merged into master. In the past we've run into situations where a "fix" on one server will break support on others. The review process is intended to prevent that or at least catch it before it gets pushed out to non-developer users.

Things might change in the future when the project has more automated tests in place, but for now please don't push things without talking to anyone else first.

ps: If you ever need to get in touch, the #OpenKore IRC channel on freenode has been pretty active lately.

@windhamwong
Copy link
Contributor

nice. I will join irc later today.
I will just create a twRO branch for updating changes here.

@lututui
Copy link
Member

lututui commented Oct 12, 2016

Redirected "Current Development" link to pull requests

http://openkore.com/index.php?title=Template:Index/MainLinks&curid=963&diff=3698&oldid=3687

@OpenKore OpenKore deleted a comment from parasquid Jul 9, 2017
allanon pushed a commit that referenced this issue Aug 1, 2017
alisonrag pushed a commit that referenced this issue Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants