Re: [asterisk-dev] managerEvent XML documentation
On Fri, Aug 1, 2014 at 5:47 PM, Corey Farrell g...@cfware.com wrote:
Hello everyone,
As part of r3811 I'm fixing build_tools/get_documentation so it picks
up every /*** DOCUMENTATION ***/ block, instead of just the first from
each file. As a side-effect this picked up some validation error's
that were previously unnoticed. One of them is in main/logger.c, the
LogChannel event documentation. This event has two variations -
Enable: yes, and Enable: no\nReason: %d - %s. Personally I feel
it would be better if we had separate LogChannelStart and
LogChannelStop events, but I suspect it's too late for that. Assuming
we keep the single event with two variations, how should it be
documented?
https://reviewboard.asterisk.org/r/3811/
Note: although the fix to build_tools/get_documentation could apply to
11/12, this caused the build to fail due to many documentation
validation errors. I'm not against eventually back-porting the fix,
but for now I'm proposing it for trunk only as I don't have time to
audit/fix/test the documentation in those versions.
I commented on the review, but I'll re-post here as well:
{quote}
Keep in mind that they are documented in two places because the same
event is raised under multiple conditions. Note the different synopsis
elements:
synopsisRaised when a logging channel is re-enabled after a reload
operation./synopsis
synopsisRaised when a logging channel is disabled./synopsis
Additionally, there is an additional field when the LogChannel event
is raised when being disabled (Reason).
The Python scripts combine these into a single event documentation
element. You can see this in the Asterisk 11 event documentation:
https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+ManagerEvent_LogChannel
Whether or not these should be renamed into two separate events is a
fair point. It's certainly the approach we've gone with for the
majority of events in AMI 2.X, and for any new events that are added.
However, splitting these up now would be a breaking change to the API.
If we're going to go do that, we should (a) do it in Asterisk 14, and
(b) make sure that we do one comprehensive change.
{quote}
I'm not sure how you would be getting documentation validation errors.
I suspect it is because you aren't combining the
managerEventInstance elements into children of a managerEvent. The
python script(s) do this and the XML document generated is valid (and
validated), so I don't think the issue is in the code.
There's a reason why I went with a Python script over awk! :-)
--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com http://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] managerEvent XML documentation
I'm not getting validation error's after the changes I've made. I was
getting validation error's from the many managerEventInstance which
were missing the class attributes. Many of those same sections that
were missing class were also not contained within a managerEvent tag.
One thing I'm unsure of, is it actually important that both
LogChannel managerEventInstance tags be in a single managerEvent
tags? Having two managerEvent with the same name seems like something
that the documentation generator should deal with (I'm guessing if
XPath is used it would be automatic).
On Wed, Aug 6, 2014 at 12:20 PM, Matthew Jordan mjor...@digium.com wrote:
On Fri, Aug 1, 2014 at 5:47 PM, Corey Farrell g...@cfware.com wrote:
Hello everyone,
As part of r3811 I'm fixing build_tools/get_documentation so it picks
up every /*** DOCUMENTATION ***/ block, instead of just the first from
each file. As a side-effect this picked up some validation error's
that were previously unnoticed. One of them is in main/logger.c, the
LogChannel event documentation. This event has two variations -
Enable: yes, and Enable: no\nReason: %d - %s. Personally I feel
it would be better if we had separate LogChannelStart and
LogChannelStop events, but I suspect it's too late for that. Assuming
we keep the single event with two variations, how should it be
documented?
https://reviewboard.asterisk.org/r/3811/
Note: although the fix to build_tools/get_documentation could apply to
11/12, this caused the build to fail due to many documentation
validation errors. I'm not against eventually back-porting the fix,
but for now I'm proposing it for trunk only as I don't have time to
audit/fix/test the documentation in those versions.
I commented on the review, but I'll re-post here as well:
{quote}
Keep in mind that they are documented in two places because the same
event is raised under multiple conditions. Note the different synopsis
elements:
synopsisRaised when a logging channel is re-enabled after a reload
operation./synopsis
synopsisRaised when a logging channel is disabled./synopsis
Additionally, there is an additional field when the LogChannel event
is raised when being disabled (Reason).
The Python scripts combine these into a single event documentation
element. You can see this in the Asterisk 11 event documentation:
https://wiki.asterisk.org/wiki/display/AST/Asterisk+11+ManagerEvent_LogChannel
Whether or not these should be renamed into two separate events is a
fair point. It's certainly the approach we've gone with for the
majority of events in AMI 2.X, and for any new events that are added.
However, splitting these up now would be a breaking change to the API.
If we're going to go do that, we should (a) do it in Asterisk 14, and
(b) make sure that we do one comprehensive change.
{quote}
I'm not sure how you would be getting documentation validation errors.
I suspect it is because you aren't combining the
managerEventInstance elements into children of a managerEvent. The
python script(s) do this and the XML document generated is valid (and
validated), so I don't think the issue is in the code.
There's a reason why I went with a Python script over awk! :-)
--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com http://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
[asterisk-dev] managerEvent XML documentation
Hello everyone, As part of r3811 I'm fixing build_tools/get_documentation so it picks up every /*** DOCUMENTATION ***/ block, instead of just the first from each file. As a side-effect this picked up some validation error's that were previously unnoticed. One of them is in main/logger.c, the LogChannel event documentation. This event has two variations - Enable: yes, and Enable: no\nReason: %d - %s. Personally I feel it would be better if we had separate LogChannelStart and LogChannelStop events, but I suspect it's too late for that. Assuming we keep the single event with two variations, how should it be documented? https://reviewboard.asterisk.org/r/3811/ Note: although the fix to build_tools/get_documentation could apply to 11/12, this caused the build to fail due to many documentation validation errors. I'm not against eventually back-porting the fix, but for now I'm proposing it for trunk only as I don't have time to audit/fix/test the documentation in those versions. -- _ -- 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
