Attention is currently required from: neels, laforge, fixeria, pespin.
7 comments:
File include/osmocom/bsc/bts_setup_ramp.h:
Patch Set #3, Line 31: BTS_SETUP_RAMP_INIT, /** initial state */
I am surprised it took so many iterations, but still does not match what Neels actually suggested. […]
One is just wrong. doxygen doesn't accept a single one. Or maybe you're using a different version or config of doxygen.
It's either /*! (java doc) or /**< but not /*< according
https://www.doxygen.nl/manual/docblocks.html#specialblock
I've tested it with doxygen using the config from libosmocore.
I've switched to /*!, but honestly we should have a linter yelling here as well.
File src/osmo-bsc/bsc_vty.c:
Patch Set #4, Line 3100: bts-setup-ramping-step-interval
I am sorry, but no. This should be addressed before merging.
As pespin proposed I followed the example of access-control-class-ramping and be consistent with access-control-class-ramping. I don't want to do more review rounds with different taste and style between different reviewers.
A proper solution for this would be a vty style guide including good and bad examples.
File src/osmo-bsc/bts_setup_ramp.c:
Patch Set #4, Line 98: && net->bts_setup_ramp.step_size > 0
Ack
Done
Patch Set #4, Line 101: net->bts_setup_ramp.active
Ack
Done
Patch Set #4, Line 201: INIT_LLIST_HEAD(&bts->bts_setup_ramp.list);
The Linux kernel llist API has always worked in the way that you cannot llist_del() a llist_entry th […]
Maybe I missed the point. But I don't see any issue with the current implementation nor do I understand why it needs to be an explicit comment in here.
File src/osmo-bsc/bts_setup_ramp.c:
Patch Set #15, Line 51: if (llist_empty(&bts->bts_setup_ramp.list))
Why don't you check the bts->bts_setup_ramp.state against BTS_SETUP_RAMP_WAIT here? […]
good point. only possible after patchset14
Patch Set #15, Line 74: net->bts_setup_ramp.count >= net->bts_setup_ramp.step_size)
Ack
Done
To view, visit change 29788. To unsubscribe, or for help writing mail filters, visit settings.