[asterisk-dev] GitHub: One Month Update

George Joseph gjoseph at sangoma.com
Tue Jun 6 06:22:46 CDT 2023


It's been a month now since we transitioned over to GitHub so I thought I'd
give you guys an update but I'd also like to hear your thoughts on the
transition and new process.

First...A Policy Change...
When we started, we said "one commit per pull request" to keep things as
close to Gerrit as possible.  After some discussions and additional
thinking however, it looks like we can allow multiple commits per pull
request under the following condition:  The commits MUST be for a single
functional change and must be able to stand on their own, in sequence from
first to last commit.  For example, a commit that adds some core capability
to JSON processing, then a second commit that updates an app or function to
use it.  In this case, the first commit can stand on its own and the second
depends on it so having both commits in the same PR would make sense.
Another good example might be a large scale change required by a new gcc
version where the files changed in the apps directory might be in one
commit and the files changed in funcs in another.

What's not acceptable are commits added to a PR that fix issues in other
commits in the same PR.  This would result in a commit that we know to
be broken making it into the git history.  An unknowing person might
checkout that commit and not realize that the following commit is also
required.  It would also make git-bisects troublesome.  Also not acceptable
are merge or other housekeeping commits.  I've seen a few merge commits
just in the past week.

Second...A Reminder...
Please keep an eye on your commit messages.  Don't put anything extraneous
after the Fixes/Resolves, UserNote or UpgradeNote trailers.  Keep all
legacy ASTERISK- mentions, Gerrit links, Reported-by and other stuff  above
all the new trailers.  It would also help me a great deal if you could
update the pull request description to match the commit message if you
happen to fix any of these issues after the PR is created.  We don't parse
the PR description at all but if I see issues in it I'll have to keep
drilling down to the commits to see if you've fixed them there.

Third...Merges...
We had some issues yesterday where two PRs were merged that both updated
Alembic scripts.  Each PR on its own passed all the tests and merge
conflict detection but when actually merged broke the previous/next links
between Alembic scripts.  This happened because we do most of the merge
checks when PRs are submitted and none just before they're actually
merged.  GitHub isn't really set up to do pre-merge checks like Gerrit did
and assumes that if a PR could be merged without git conflict, it must be
good.  We're looking at ways to solve this issue which may include a new
"Merge Queue" feature GitHub has in beta testing.  It might take a few
weeks to iron the kinks out though.  In the mean time, we might
automatically add an "ALEMBIC CHANGE!" flag to any PR that touches the
Alembic scripts just to make us aware that there might be a conflict.

Finally...Your Thoughts?
How's everything working from your perspective?  Anything we should change?

-- 
George Joseph
Asterisk Software Developer
Sangoma Technologies
Check us out at www.sangoma.com and www.asterisk.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20230606/29337084/attachment.html>


More information about the asterisk-dev mailing list