Re: [asterisk-dev] GitHub: Side Note: What makes us "special"?
On Tue, Apr 4, 2023 at 5:27 PM George Joseph wrote: > > > On Tue, Apr 4, 2023 at 1:16 PM wrote: > >> On 4/4/2023 2:53 PM, George Joseph wrote: >> > >> > >> > Speaking of workflows... If you want to see the workflows and >> > actions we've written so far, check out the asterisk/asterisk-gh-test >> (the >> > .github/workflows directory) and asterisk/asterisk-ci-actions repos. >> If >> > you're experienced with GitHub workflows, feedback is appreciated. >> Thanks, George, et al, for all this amazing work. I admit Gerrit has >> grown on me a little over the years, but other developers I've discussed >> with do prefer GitHub and I'm eager to give this a try when it's all >> ready. >> >> One question from looking through some of the workflows that are up now: >> >> https://github.com/asterisk/asterisk-gh-test/blob/master/.github/workflows/CloseStaleIssues.yml >> >> I'm a bit curious about the auto-closing functionality: >> > >> * Do you think 14-21 days is a sufficient threshold for most issues? >> It seems potentially a bit low to me. For example, once an issue is >> triaged and opened, will it just be closed automatically 3 weeks >> later if it hasn't been resolved by then? Or are issues in the >> 'open' state exempt from this, this is purely for triage to weed out >> junk issues? >> > * Case in point: one vendor I deal with frequently has this annoying >> auto-close functionality in their system which triggers after about >> 2 weeks or so. Often more time is required on one of our ends just >> to follow up on the last thing, so there is a lot of inevitable >> "commenting to avoid auto closure" and this just adds a lot of noise >> into the tickets. >> * Is there any connection with reviews/PRs in progress? Suppose an >> issue is open and maybe on the verge of being stale, but someone has >> submitted a PR against. Changes can often take much longer than 3 >> weeks to merge, so it wouldn't make sense for an issue to close >> itself in that case. So I'm concerned perhaps that might not be >> sufficient time. >> > > We're still thinking about the issues process but... > > The action allows you to specify labels that make an issue exempt from > auto-closure. I was thinking that when a PR gets submitted, we'd look for > the "Resolves: #issuenum" tag in the commit message, then add an > "InProgress" label to the issue to prevent it from being auto closed. The > issue would then get closed when the PR is closed. > > I'm also thinking it would only close issues that have been inactive and > assigned to the submitter. Like the "Waiting for Feedback" status in Jira. > > Does that make sense? > I think issues should only be closed if we are waiting for feedback from the reporter during the triage process. Once accepted the issue should remain open until either automatically closed through PR, or someone else closes it. Same as now. -- 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: Side Note: What makes us "special"?
On 4/4/2023 4:26 PM, George Joseph wrote: On Tue, Apr 4, 2023 at 1:16 PM wrote: On 4/4/2023 2:53 PM, George Joseph wrote: * Is there any connection with reviews/PRs in progress? Suppose an issue is open and maybe on the verge of being stale, but someone has submitted a PR against. Changes can often take much longer than 3 weeks to merge, so it wouldn't make sense for an issue to close itself in that case. So I'm concerned perhaps that might not be sufficient time. We're still thinking about the issues process but... The action allows you to specify labels that make an issue exempt from auto-closure. I was thinking that when a PR gets submitted, we'd look for the "Resolves: #issuenum" tag in the commit message, then add an "InProgress" label to the issue to prevent it from being auto closed. The issue would then get closed when the PR is closed. I'm also thinking it would only close issues that have been inactive and assigned to the submitter. Like the "Waiting for Feedback" status in Jira. Does that make sense? That makes sense, that seems like it would replicate the current behavior pretty nicely. Thanks, George! -- _ -- 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: Side Note: What makes us "special"?
On Tue, Apr 4, 2023 at 1:16 PM wrote: > On 4/4/2023 2:53 PM, George Joseph wrote: > > > > > > Speaking of workflows... If you want to see the workflows and > > actions we've written so far, check out the asterisk/asterisk-gh-test > (the > > .github/workflows directory) and asterisk/asterisk-ci-actions repos. If > > you're experienced with GitHub workflows, feedback is appreciated. > Thanks, George, et al, for all this amazing work. I admit Gerrit has > grown on me a little over the years, but other developers I've discussed > with do prefer GitHub and I'm eager to give this a try when it's all ready. > > One question from looking through some of the workflows that are up now: > > https://github.com/asterisk/asterisk-gh-test/blob/master/.github/workflows/CloseStaleIssues.yml > > I'm a bit curious about the auto-closing functionality: > > * Do you think 14-21 days is a sufficient threshold for most issues? > It seems potentially a bit low to me. For example, once an issue is > triaged and opened, will it just be closed automatically 3 weeks > later if it hasn't been resolved by then? Or are issues in the > 'open' state exempt from this, this is purely for triage to weed out > junk issues? > * Case in point: one vendor I deal with frequently has this annoying > auto-close functionality in their system which triggers after about > 2 weeks or so. Often more time is required on one of our ends just > to follow up on the last thing, so there is a lot of inevitable > "commenting to avoid auto closure" and this just adds a lot of noise > into the tickets. > * Is there any connection with reviews/PRs in progress? Suppose an > issue is open and maybe on the verge of being stale, but someone has > submitted a PR against. Changes can often take much longer than 3 > weeks to merge, so it wouldn't make sense for an issue to close > itself in that case. So I'm concerned perhaps that might not be > sufficient time. > We're still thinking about the issues process but... The action allows you to specify labels that make an issue exempt from auto-closure. I was thinking that when a PR gets submitted, we'd look for the "Resolves: #issuenum" tag in the commit message, then add an "InProgress" label to the issue to prevent it from being auto closed. The issue would then get closed when the PR is closed. I'm also thinking it would only close issues that have been inactive and assigned to the submitter. Like the "Waiting for Feedback" status in Jira. Does that make sense? > > I guess this will answer itself after the migration when we see how > people interact with it, but curious if these were just defaults or if > these were customized for the project. > > Thanks again for all this heavy lifting! > > NA > > -- _ -- 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: Side Note: What makes us "special"?
On 4/4/2023 2:53 PM, George Joseph wrote: Speaking of workflows... If you want to see the workflows and actions we've written so far, check out the asterisk/asterisk-gh-test (the .github/workflows directory) and asterisk/asterisk-ci-actions repos. If you're experienced with GitHub workflows, feedback is appreciated. Thanks, George, et al, for all this amazing work. I admit Gerrit has grown on me a little over the years, but other developers I've discussed with do prefer GitHub and I'm eager to give this a try when it's all ready. One question from looking through some of the workflows that are up now: https://github.com/asterisk/asterisk-gh-test/blob/master/.github/workflows/CloseStaleIssues.yml I'm a bit curious about the auto-closing functionality: * Do you think 14-21 days is a sufficient threshold for most issues? It seems potentially a bit low to me. For example, once an issue is triaged and opened, will it just be closed automatically 3 weeks later if it hasn't been resolved by then? Or are issues in the 'open' state exempt from this, this is purely for triage to weed out junk issues? * Case in point: one vendor I deal with frequently has this annoying auto-close functionality in their system which triggers after about 2 weeks or so. Often more time is required on one of our ends just to follow up on the last thing, so there is a lot of inevitable "commenting to avoid auto closure" and this just adds a lot of noise into the tickets. * Is there any connection with reviews/PRs in progress? Suppose an issue is open and maybe on the verge of being stale, but someone has submitted a PR against. Changes can often take much longer than 3 weeks to merge, so it wouldn't make sense for an issue to close itself in that case. So I'm concerned perhaps that might not be sufficient time. I guess this will answer itself after the migration when we see how people interact with it, but curious if these were just defaults or if these were customized for the project. Thanks again for all this heavy lifting! NA -- _ -- 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: Side Note: What makes us "special"?
Every open source project, CI/CD system and SCM system has its own quirks. Asterisk and GitHub are no exception. :)Here's some background, most of which is just informational for you but caused us some head scratching and will probably continue to do so. 1. Very few GitHub projects have multiple simultaneous release branches as we do and GitHub has no built-in cherry-picking functionality. 2. For very valid security reasons, GitHub limits the permissions of workflows triggered by PRs submitted from forked repositories to read-only. Otherwise anyone could fork the Asterisk repo and submit a pull request that changes the workflow that's about to run for thiat PR. OK, it's not quite as easy as that but it is a concern. 3. Some of the automations we need, like simply reporting test completion status on the PR, require write access to the PR. 4. We could add Asterisk community developers as collaborators to the repos which would give them additional permissions but that becomes an administrative overhead for the core team. Besides... 5. GitHub's most restrictive level of collaborator access (Triager) allows a user to manipulate the PRs and issues belonging to other users which is probably not a good idea. 6. You know how you can add a "regate" or "recheck" comment to Gerrit today and have Jenkins re-run the tests? Well, GitHub doesn't need that because it has the ability to re-run jobs right from the UI. However, when we were thinking about the cherry-pick process we thought we could trigger it using the same mechanism...just add the comment and the process would kick off. Unfortunately, unlike Gerrit/Jenkins, if you have a job trigger on a comment, it'll trigger on EVERY comment even if the keyword isn't present in it. That's just a waste of resources and it would flood the job history with crap. Then we thought... 7. In my earlier email I mentioned an Asterisk core team member having to add a label to kick off the cherry-pick process. Well, that started with "Let's have the user add labels to kick the cherry-pick process off". Except... A user who is not a member of the organization can't add labels even to their own PRs and issues. That's just some of the background that's driving the process and development of the workflows. Speaking of workflows... If you want to see the workflows and actions we've written so far, check out the asterisk/asterisk-gh-test (the .github/workflows directory) and asterisk/asterisk-ci-actions repos. If you're experienced with GitHub workflows, feedback is appreciated. -- _ -- 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] Call transfer (302 Moved temporarily) not working with pjsip
On Tue, Apr 4, 2023 at 2:17 PM Joshua C. Colp wrote: > On Tue, Apr 4, 2023 at 2:11 PM Karsten Wemheuer wrote: > >> >> I filed an issue about this. No one has worked on the issue yet, so I >> would start with this. Can anyone help me get started? >> >> > You'd need to be specific about what you are seeking help with. The 302 > code is in res_pjsip_diversion.c, NAT handling is in res_pjsip_nat.c. There > are instructions on the wiki[1] for Gerrit to put things up for code review. > > Sorry, this is the outgoing 302 case which is in chan_pjsip.c -- 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] Call transfer (302 Moved temporarily) not working with pjsip
On Tue, Apr 4, 2023 at 2:11 PM Karsten Wemheuer wrote: > > I filed an issue about this. No one has worked on the issue yet, so I > would start with this. Can anyone help me get started? > > You'd need to be specific about what you are seeking help with. The 302 code is in res_pjsip_diversion.c, NAT handling is in res_pjsip_nat.c. There are instructions on the wiki[1] for Gerrit to put things up for code review. [1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage -- 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] Call transfer (302 Moved temporarily) not working with pjsip
Hi *, Am Donnerstag, dem 02.03.2023 um 13:05 -0400 schrieb Joshua C. Colp: > On Thu, Mar 2, 2023 at 1:00 PM Karsten Wemheuer wrote: > > Hi Joshua, > > > > Am Donnerstag, dem 02.03.2023 um 12:44 -0400 schrieb Joshua C. > > Colp: > > > On Thu, Mar 2, 2023 at 12:37 PM Karsten Wemheuer > > > wrote: > > > > Hi Joshua, > > > > > > > > thank You, for answering. > > > > > > > > Am Donnerstag, dem 02.03.2023 um 09:17 -0400 schrieb Joshua C. > > > > Colp: > > > > > On Thu, Mar 2, 2023 at 9:04 AM Karsten Wemheuer > > > > > wrote: > > > > > > Hi *, > > > > > > > > > > > > Maybe I found a small bug or I am doing something wrong. > > > > > > > > > > > > When I do a "Transfer" on a call that arrives via PJSIP, > > > > > > Asterisk sends > > > > > > a "302 Moved Temporarily" to perform the transfer. > > > > > > > > > > What version of Asterisk? What is the precise transport > > > > > configuration? > > > > > > > > As written below it was Version 18. The exact version is > > > > 18.16.0. > > > > > > > > > > In the future please always provide the precise Asterisk version. > > > It's important, as code changes. > > > > > > What is the precise transport configuration in PJSIP? > > > > > > > Transport section is below: > > > > [transport-tcp] > > type = transport > > protocol = tcp > > bind = 0.0.0.0:25060 > > external_media_address = 91.2.166.143 > > external_signaling_address = 91.2.166.143 > > local_net = 10.0.1.0/24 > > local_net = 192.168.10.0/24 > > local_net = 169.254.0.0/24 > > tos = 96 > > allow_reload = no > > > > Then the Contact replacement for NAT purposes may not be specific > enough. File an issue[1] however ALSO include a full SIP trace. > > [1] https://issues.asterisk.org/jira I filed an issue about this. No one has worked on the issue yet, so I would start with this. Can anyone help me get started? [1] https://issues.asterisk.org/jira/browse/ASTERISK-30451 -- _ -- 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: Proposed Pull Request Process
I think publishing this on the dev list before the wiki will make it easier for people to read, quote and respond,so here goes... Since most of you have used GitHub, the first part of this process should be familiar. Before we get into it though, you should install the GitHub CLI (gh). Most distributions have a package for it. Although not strictly required, it'll make life easier. I'm using "asterisk" as the example but the same applies to the "testsuite" repo as well. 1. Fork the asterisk/asterisk repo into your own account. 2. Clone the forked repo to your local dev environment: `gh repo clone you/asterisk` The gh tool will automatically create an "upstream" remote that points back to asterisk/asterisk as well as the "origin" remote pointing to your fork. 3. Set the default remote repository: `gh repo set-default` When prompted, select "asterisk/asterisk" which should be the default choice. 4. Checkout a branch, do some work and commit as usual. I'll use "master" as an example. We've got some additional commit message formatting rules that do away with the need to create entries in the doc/CHANGES-staging and doc/UPGRADE-staging directories. More on this later. 5. Create a pull request with: `gh pr create --fill --base master` Do NOT cherry-pick to any other branches at this time. 6. When you submit your first pull request, you'll need to open the PR in a browser, scroll down to the checks area, click the "details" link for the "license/cla" entry and complete a new license agreement. At this point, things become a bit "special" because of the way GitHub operates and the fact that we maintain multiple simultaneous release branches. Testing: With Gerrit, when you submit a new review, the unit tests run but the gate tests don't run until we do a '+2'. With the new process, we have more resources at our disposal so we run both the unit and gate tests immediately. You'll notice a whole bunch of entries in the PR checks area for them and you can easily click through to see the details. Right now, the output is overly verbose but we'll pare that down over time. Addressing test failures or review feedback: I'll cover the actual review process later but if you need to make changes, the process will be much like it is today (at first). While Gerrit has a one-to-one relationship between commit and review, GitHub allows a PR to contain multiple commits. For the time being though, we'd like to keep the one-to-one relationship to keep things simple. This means making your changes locally and doing a `git commit --amend` as you do today but then doing a `git push --force` to update the PR instead of a `git review`. There's one exception to this rule though. See below. Cherry-picking: This is where things really get "special". If you've worked with GitHub you know that, unlike Gerrit, they have no built-in support for cherry-picking but we've come with a way to automate most of the process: 1. Once your PR is created, simply add a comment indicating which other branches you'd like the PR cherry-picked to. If the PR was submitted against the master branch and you'd like it cherry-picked to the 18 and 20 branches, you'd add a comment as follows: Comment: cherry-pick-to: 18 cherry-pick-to: 20 2. When you get your first "+1" review, a core team member will add a label to the PR that will trigger test cherry-picks to the branches you indicated and run the unit and gate tests on each. 3. If there are no merge conflicts or test failures then when the base PR merges, the cherry-picks will automatically merge to their respective branches. 4. If code changes are required for a specific branch, you'll need to edit the comment listing the cherry-pick branches and remove the offending entry, then submit a completely separate PR for that branch. The exception to the one-to-one rule: When you have a series of patches that depend on each other, Gerrit detects this and only creates new reviews for the new commits and uses the earlier in-progress reviews as the base for them. GitHub does things a bit different depending on how you push up the new work. If you create a new commit in the same branch as the first one and do a simple `git push`, the new commit will be added to the existing PR. If you instead do a `gh pr create`, a new PR will be created that has both the new and earlier commits. This can make things a bit confusing and we're not quite sure how to deal with it yet. Stay tuned. Comments? Questions? Despite everything written above, I think that for the majority of contributions, the GitHub process will actually be easier than the current Gerrit process. -- -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit:
