My first bug fix posted

quartarian wrote on Wednesday, March 27, 2013:

Hey guys, I posted my first bug fix over here:
https://github.com/quartarian/openemr/tree/firstbugfix

This has two changes:
1. I fixed two mistyped variables
2. Lowered the font-size for print version of Appointments Report.

Let me know if I did everything OK.

-Matt

yehster wrote on Wednesday, March 27, 2013:

I scanned the appointment reports script briefly, and I think there might be a couple more instances of the wrong variable name being used in the original code.
Otherwise, looks Ok.

sunsetsystems wrote on Wednesday, March 27, 2013:

Thanks for the fix.  Yehster’s observation seems correct, one more name correction needed.  Also I added a couple of minor comments on github.

Rod
www.sunsetsystems.com

quartarian wrote on Wednesday, March 27, 2013:

Thanks guys! I replied to your comments on GitHub and submited a new commit.

-Matt

yehster wrote on Monday, April 01, 2013:

Matt,
Thank you for the contribution.  Your code changes have been incorporated into the master branch.  Looking forward to your next submission.
-Kevin

quartarian wrote on Monday, April 01, 2013:

Woot! That’s awesome!

Also thanks for the great explanation of the squash rebase. I did it a few times for practice.

Quick question; now that the code has been pulled, what is considered best practice for branch managment on my end? Should I deleted the my “firstbugfix” branch, or should I leave it for posterity?

Thanks again!

yehster wrote on Monday, April 01, 2013:

When dealing with my own branches, if it’s a complicated branch, I typically don’t delete it right away after a merge.   Somehow I feel like it’s useful to hang onto it for a while.  However, I typically end up deleting it later anyway. 

An interesting thing to look at is branch management in github.
https://github.com/yehster/openemr/branches
From this link, you will see another link that says View merged branches.
On the merged branches list, appt_report_fixes now shows up. Since that’s the branch I used to actually push the changes into github.
And the contents of that branch are now part of the master.
However,
https://github.com/yehster/openemr/tree/rebase_example
that I used to explain rebase is still listed as “unmerged.”

Comments that may have been made on a given branch are lost when a branch is deleted, so that one good reason NOT to delete a branch.

quartarian wrote on Tuesday, April 02, 2013:

OK, that makes perfect sense. I’ll definitely leave that branch up then as I’m sure I’ll be using your comment as a referance for a while :slight_smile:

bradymiller wrote on Tuesday, April 02, 2013:

Hi Matt,

There are several styles of how to do this ranging from don’t ever remove anything to keep it clean and tidy. As examples, check out Rod’s(Sunset Systems) which is neat and tidy and mine which never deletes anything (and Kevin’s lies somewhere in between):
http://github.com/sunsetsystems/openemr/branches
http://github.com/yehster/openemr/branches
http://github.com/bradymiller/openemr/branches

So, you can pretty much do whatever you want. Main thing to do is to not remove branches that have been reviewed until the feature is accepted; for example, if you get your code reviewed and do some work on it and then rebase it and submit it for another review it’s important to ensure the old review still exists so the reviewer can reference it. Do note that even though you remove a branch on github that the commit will always stay there (the only way to really remove anything on github is to completely delete your github repo and install it again from your local repo).

BTW Kevin, I never noted the View Merged Branch link; that is very useful.

-brady
OpenEMR