Comments (9)
Hi, there doesn't seems to be much reviewing going here, and I thought maybe the way I've opened PRs sporadically has made it difficult to follow.
I opened issues #43, #44, and #46, of which you've closed the first one, but one of the bugs reported in there hasn't been fixed.
-
PR #42
0261230 (unecessary now 2ce03eb has been committed to fix the same bug.)
b6c46fc (referenced in issue #44.) -
PR #45
5f98a35 (will fix an unresolved bug reported in issue #43.)
94d0357 (referenced in #46)
I can close all these PRs and rebase the remaining 4 commits to my local master for which to open a new PR, if you think it'll be tidier and easy to review.
Pls let me know. I want to deploy this LG for my team, but don't want to maintain the code locally. Thanks.
from bird-lg.
Sorry for the delay in reviewing, I will have a look at these PRs soon.
from bird-lg.
@tamihiro Thanks for submitting these patches. Here is a quick review, I'm a bit confused:
- I don't understand 94d0357, it touches both request parsing/printing and graphical display code. Can you instead do two commits, each with a commit message that explains the problem and the solution?
- #47 looks like it will introduce a regression. In
show route for XXX all
all AS numbers inBGP.as_path
are clickable, is that still the case after your commit? What problem are you trying to solve? - I still need to test #44, I will try to do that at some point
- I think the remaining issue in #43 is now fixed thanks to #48, can you confirm?
from bird-lg.
@zorun Thanks for the reply. First let me answer your questions.
I don't understand 94d0357, it touches both request parsing/printing and graphical display code. Can you instead do two commits, each with a commit message that explains the problem and the solution?
These parsing and printing are pretty much coupled, but I'll see if I can try. Anyway please read on.
#47 looks like it will introduce a regression. In show route for XXX all all AS numbers in BGP.as_path are clickable, is that still the case after your commit?
Let me put it this way.
The output of show route for XXX all
includes AS numbers:
[o] on the right side of BGP.as_path:
that would match r'\d+'
, as you've suggested, and also
[o] on the via nexthop...
lines, inside square brackets, that would match r'AS\d+'
.
The output of show protocols <protocol_name> for all
includes:
[o] AS numbers on the right side of Neighbor AS:
that would match r'\d+'
, and also
[x] 4byte AS capability on the right side of Session:
that would match r
\bAS4\b'`.
What problem are you trying to solve?
The pattern in the last one marked with [x] should not be clickable, and I'm trying to solve it.
You can see it here for instance.
Here, AS4
should not be treated as ASN because it's about capability.
I think the remaining issue in #43 is now fixed thanks to #48, can you confirm?
Unfortunately not quite..
So.. at this point my PRs have made you confused, for which I'm sorry. Honestly I myself is confused too.
For clarity's sake, if you'd like, I'll be happy to drop all the PRs (#42, #45, #47) for now.
Then I'd pull your current master (#48 included), and on top of it write patches and resubmit a PR one by one.
Please consider and answer my suggestion at your earliest convenience. Thank you.
from bird-lg.
Thanks for the explanation, new PRs sound good! Make sure to include what you just explained in the commit messages :)
from bird-lg.
@zorun, I have dropped all the previous PRs, and resubmitted #49, #50, #51, #52, and #53.
Each one intends to correct an individual bug, so please review and confirm one by one. Each commit message hopefully gives you enough explanation.
from bird-lg.
@zorun it's been a while so I've just thought if there's anything that's still unclear. If there is pls feel free to ask.
from bird-lg.
@tamihiro yes, sorry for the delay, I have started reviewing the new PR!
from bird-lg.
It seems that some of your PR slipped through the cracks again :(
I have merged all of them, thanks again for your perseverance!
from bird-lg.
Related Issues (20)
- bird-2.0.x compatibility HOT 2
- AS path prepend count is incorrect in the map HOT 5
- BIND_ADDR and BIND_PORT ignored when run by "flask run" HOT 2
- self xss in every page HOT 3
- TypeError: unsupported operand type(s) for +: 'NoneType' and 'str' HOT 4
- Looking for new maintainers / project reboot HOT 7
- Openly-accessible whois execution
- Stop using DOMAIN in the templates
- Removing dependency on memcached?
- Provide docker images HOT 1
- Feature request - default webpage & disable IPv6 or IPv4 HOT 2
- Add AngolaCables to Happy users section HOT 2
- Add a public-facing API HOT 2
- New release with backwards-incompatible changes
- Add python3 support HOT 3
- BGPMAP is non-deterministic, visually
- auto detect AFI for "show route for" query
- Cross-site Scripting (XSS) vulnerability HOT 2
- How connect to router
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from bird-lg.