[asterisk-dev] GitHub: One Month Update

[Ross Beer]

Hi George,

First time using GitHub here and it looks like I need to re-sign the contributor license agreement. This is approved in Jira, how do we go about signing this in GitHub?

I'm sure others will have the same issue so thought it was pertinent to reply to this thread.

Regards,

Ross
________________________________
From: asterisk-dev <asterisk-dev-bounces at lists.digium.com> on behalf of George Joseph <gjoseph at sangoma.com>
Sent: 06 June 2023 12:22
To: Asterisk Developers Mailing List <asterisk-dev at lists.digium.com>
Subject: [asterisk-dev] GitHub: One Month Update

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<http://www.sangoma.com/> and www.asterisk.org<http://www.asterisk.org/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20230606/52af2876/attachment.html>