Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added documentation for generated files #1494

Merged
merged 7 commits into from
Aug 31, 2020

Conversation

ethouris
Copy link
Collaborator

No description provided.

@ethouris ethouris self-assigned this Aug 20, 2020
@ethouris ethouris added [docs] Area: Improvements or additions to documentation Impact: Development Priority: High Type: Maintenance Work required to maintain or clean up the code labels Aug 20, 2020
@ethouris ethouris added this to the v1.5.0 - Sprint 21 milestone Aug 20, 2020
ethouris pushed a commit to ethouris/srt that referenced this pull request Aug 20, 2020
docs/Developers.md Outdated Show resolved Hide resolved
Co-authored-by: stevomatthews <smatthews@haivision.com>
Comment on lines 145 to 148
The fine-grained functional areas in the logging system allows the developer to
turn on selectively only some smaller areas so that the logging doesn't overflow
the performance and change the behavior when testing, in case when heavy debug
logging is turned on.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The fine-grained functional areas in the logging system allows the developer to
turn on selectively only some smaller areas so that the logging doesn't overflow
the performance and change the behavior when testing, in case when heavy debug
logging is turned on.
The fine-grained functional structure of the logging system allows a developer to
selectively turn on only specific areas. This is useful during testing to help minimize the impact of logging on performance or behavior.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"functional area" term should remain used here. Maybe it should be phrased in the sentence a little different.

Copy link
Collaborator

@stevomatthews stevomatthews Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:
"The logging system has functional areas (FA) that allow a developer to
selectively turn on only specific types of logs. This is ..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, still not sure that this sounds clear.

The logging system has two aspects for every log, which is level and FA. Every logging instruction looks more-less like this:

LOGC(cclog.Note, log << "Here is the note");

where:
cclog - the logger variable that defines the FA. Here cc means "Congestion Control".
Note - the log level, one of: Debug, Note, Warn, Error, Fatal.

By setting the log level, you allow only logs of given level and above to be printed. By seting on/off particular FA, you allow logs only for allowed FA to be printed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your version seems to be clear enough, though. I'll try to add only some more information to this sentence to clarify.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:
"In addition to levels (Debug, Note, Warn, Error, Fatal) the logging system has functional areas (FA) that allow a developer to
selectively turn on only specific types of logs. For example, in this logging instruction:

LOGC(cclog.Note, log << "Here is the note");

cclog is a logger variable that defines the FA (Congestion Control). And Note defines the log level. By setting an FA, you allow logs only for that FA to be printed. By setting the log level, you allow only logs of that level and above to be printed. This is ..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've tried to make some common version out of it all. Please re-check.

docs/Developers.md Outdated Show resolved Hide resolved
docs/Developers.md Outdated Show resolved Hide resolved
Co-authored-by: stevomatthews <smatthews@haivision.com>
Comment on lines 163 to 164
* Note that `srt.inc.h` file generates only a part that you have to place in srt.h
* Find any `SRT_LOGFA_` symbol to find the place where it should be pasted
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Note that `srt.inc.h` file generates only a part that you have to place in srt.h
* Find any `SRT_LOGFA_` symbol to find the place where it should be pasted
* The script generates `srt.inc.h` with the parts you have added or changed (e.g. `#define SRT_LOGFA_GENERAL 0 // glog`)
* Copy any new definitions from `srt.inc.h` into `srt.h` at the appropriate place (search for `SRT_LOGFA_`)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this makes it clearer. The srt.inc.h file is not really a part of generated files - this exceptionally contains only a "generated part" that shouild be pasted into srt.h file. The 3 others are fully generated files.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please show where to find srt.inc.h? It will help me to understand better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tell you what. Forget it. I decided to change things here. Now all files will be generated.

* entry format for `hidden_loggers` (optional)

The 'format model' uses two variables: `globalheader`, which is the obligatory
header for all source files, and `entries` in a place where the list of FA entries
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "FA"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional Area.


If you modify the `CMakeLists.txt` file and add some build options to it, remember to
generate this list of options and update appropriately the `configure-data.tcl` file.
This file requires to be run in the build directory and passed the `CMakeCache.txt`
Copy link
Collaborator

@stevomatthews stevomatthews Aug 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This file requires to be run in the build directory and passed the `CMakeCache.txt`
This file must be run in the build directory and passed the `CMakeCache.txt`

Comment on lines 210 to 214
Note that it doesn't mean that the contents should be blindly pasted into the
options list, just apply only these new options that you have added - this
script does its best to make sure that no option is missing, but some of them
could be provided by some foreign dependent script (like `build-gmock`) and
therefore they are mistakenly added to the list.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note that it doesn't mean that the contents should be blindly pasted into the
options list, just apply only these new options that you have added - this
script does its best to make sure that no option is missing, but some of them
could be provided by some foreign dependent script (like `build-gmock`) and
therefore they are mistakenly added to the list.
Note that this does not mean that the contents should be blindly pasted into the
options list. Apply only the new options that you have added. The
script does its best to make sure that no option is missing. Note that some options might be provided by an external dependent script (like `build-gmock`) and
therefore mistakenly added to the list.

Copy link
Collaborator

@stevomatthews stevomatthews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor edits.

* entry format for `hidden_loggers` (optional)

The 'format model' uses two variables: `globalheader`, which is the obligatory
header for all source files, and `entries` in a place where the list of FA entries
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
header for all source files, and `entries` in a place where the list of FA entries
header for all source files, and `entries` in a place where the list of functional area (FA) entries

Copy link
Collaborator

@stevomatthews stevomatthews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor edit

@maxsharabayko maxsharabayko modified the milestones: v1.5.0 - Sprint 21, v1.5.0 - Sprint 22 Aug 25, 2020
Copy link
Collaborator

@stevomatthews stevomatthews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good.

docs/Developers.md Outdated Show resolved Hide resolved
docs/Developers.md Outdated Show resolved Hide resolved
Co-authored-by: Maxim Sharabayko <maxlovic@gmail.com>
@maxsharabayko maxsharabayko merged commit a755d8f into Haivision:master Aug 31, 2020
@mbakholdina mbakholdina modified the milestones: v1.5.0 - Sprint 22, v1.4.2 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[docs] Area: Improvements or additions to documentation Priority: High Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants