Re: [asterisk-dev] GitHub: One Month Update
On Tue, Jun 6, 2023 at 5:59 AM Ross Beer wrote: > 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? > If you look in the status section of the PR and scroll to the "license/cla" entry, you can click "Details" and it'll take you to the CLA manager where you can sign. Acceptance is immediate. > > I'm sure others will have the same issue so thought it was pertinent to > reply to this thread. > > Regards, > > Ross > -- > *From:* asterisk-dev on behalf of > George Joseph > *Sent:* 06 June 2023 12:22 > *To:* Asterisk Developers Mailing List > *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 and www.asterisk.org > -- > _ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: >http://lists.digium.com/mailman/listinfo/asterisk-dev > -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] GitHub: One Month Update
On Tue, Jun 6, 2023 at 8:59 AM Ross Beer wrote: > 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? > The code contribution process is documented on the wiki[1] including signing the contributor license agreement[2] which is listed under the PR checks as its own check. [1] https://wiki.asterisk.org/wiki/display/AST/Code+Contribution [2] https://wiki.asterisk.org/wiki/display/AST/Code+Contribution#CodeContribution-NewContributorLicenseAgreement -- Joshua C. Colp Asterisk Project Lead Sangoma Technologies Check us out at www.sangoma.com and www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] GitHub: One Month Update
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 on behalf of George Joseph Sent: 06 June 2023 12:22 To: Asterisk Developers Mailing List 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/> -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[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 and www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
