Conversation
polychaeta
left a comment
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/28478018ece60f0eaf048baab61abe60/raw/9469e37c46dfeec09e10a844354a3255ff700d37/cr_7435_1604232741.diff | git apply
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index f80c42e5f..3870ef35c 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -746,7 +746,7 @@ bool bgp_zebra_nexthop_set(union sockunion *local, union sockunion *remote,
peer->bgp->vrf_id);
else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
- peer->bgp->vrf_id);
+ peer->bgp->vrf_id);
} else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
peer->bgp->vrf_id);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
58c8010 to
2c2da34
Compare
polychaeta
left a comment
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/5c99a8e6751959c0051784624a5cef23/raw/9469e37c46dfeec09e10a844354a3255ff700d37/cr_7435_1604232865.diff | git apply
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index f80c42e5f..3870ef35c 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -746,7 +746,7 @@ bool bgp_zebra_nexthop_set(union sockunion *local, union sockunion *remote,
peer->bgp->vrf_id);
else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
- peer->bgp->vrf_id);
+ peer->bgp->vrf_id);
} else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
peer->bgp->vrf_id);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedFreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/CI009BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/CENTOS8BUILD/config.status/config.statusPackage building failed for CentOS 8 amd64 build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build: Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsFreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/CI009BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/CENTOS8BUILD/config.status/config.statusPackage building failed for CentOS 8 amd64 build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15099/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build: Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: FAILEDContinuous Integration Result: FAILEDSee below for issues. This is a comment from an automated CI system. Get source / Pull Request: SuccessfulBuilding Stage: FailedFreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/CI009BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/CENTOS8BUILD/config.status/config.statusPackage building failed for CentOS 8 amd64 build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build: Successful on other platforms/tests
Warnings Generated during build:Checkout code: Successful with additional warningsFreeBSD 11 amd64 build: Failed (click for details)Make failed for FreeBSD 11 amd64 build: FreeBSD 11 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/CI009BUILD/config.status/config.status CentOS 8 amd64 build: Failed (click for details)CentOS 8 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/CENTOS8BUILD/config.status/config.statusPackage building failed for CentOS 8 amd64 build Ubuntu 20.04 amd64 build: Failed (click for details)Ubuntu 20.04 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15100/artifact/U2004AMD64BUILD/config.status/config.statusMake failed for Ubuntu 20.04 amd64 build: Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
donaldsharp
left a comment
There was a problem hiding this comment.
Please rebase and force push to latest master. This PR is ~4k commits behind.
2c2da34 to
5e1464a
Compare
polychaeta
left a comment
There was a problem hiding this comment.
Thanks for your contribution to FRR!
Click for style suggestions
To apply these suggestions:
curl -s https://gist.githubusercontent.com/polychaeta/e7d425883d0300c739381c6fccce5bb3/raw/9469e37c46dfeec09e10a844354a3255ff700d37/cr_7435_1604315605.diff | git apply
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index f80c42e5f..3870ef35c 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -746,7 +746,7 @@ bool bgp_zebra_nexthop_set(union sockunion *local, union sockunion *remote,
peer->bgp->vrf_id);
else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
- peer->bgp->vrf_id);
+ peer->bgp->vrf_id);
} else if (peer->update_if)
ifp = if_lookup_by_name(peer->update_if,
peer->bgp->vrf_id);
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Outdated results 💚Basic BGPD CI results: SUCCESS, 0 tests failed
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-15114/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warningsWarnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
is this the same as #7434? |
|
once the format changes are fixed up, this looks good to me. |
5e1464a to
836c760
Compare
polychaeta
left a comment
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits has a missing or badly formatted
Signed-off-byline; we can't accept your contribution until all of your commits have one - One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
836c760 to
265ccca
Compare
polychaeta
left a comment
There was a problem hiding this comment.
Thanks for your contribution to FRR!
- One of your commits does not have a blank line between the summary and body; this will break
git log --oneline
If you are a new contributor to FRR, please see our contributing guidelines.
After making changes, you do not need to create a new PR. You should perform an amend or interactive rebase followed by a force push.
Description:
Holdtime and keepalive parameters weren't copied from
peer-group to peer-group members. Fixed the issue by copying holdtime
and keepalive parameters from peer-group to its members.
Problem Description/Summary :
Holdtime and keepalive parameters weren't copied from
peer-group to peer-group members. Fixed the issue by copying holdtime
and keepalive parameters from peer-group to its members.
Signed-off-by: sudhanshukumar22 <sudhanshu.kumar@broadcom.com>
265ccca to
b28a12e
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
|
Can you be more specific in the commit subject line? This sounds kinda fixes anything. |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16709/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULContinuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16708/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-16707/ This is a comment from an automated CI system. Warnings Generated during build:Debian 10 amd64 build: Successful with additional warningsDebian Package lintian failed for Debian 10 amd64 build: |
|
@donaldsharp : I have removed this code since it is part of another commit(#7434). please approve. |
|
@ton31337, please approve this PR. |
|
@ton31337 : Please approve this file. |
No description provided.