-
Notifications
You must be signed in to change notification settings - Fork 775
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
[20823] Example refactor: Content filter #4859
Conversation
8bdb05a
to
29d638f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job ! really nice example refactor.
Apart from the suggestions, we would need to update the README.md
of the example and pass uncrustify
Also, we would need to update |
30cb7ac
to
558593c
Compare
35872ef
to
0b0f246
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are getting there ! Leaving some comments.
In addition
- We would also take the chance for renaming
MyCustomFitler
toCustomContentFilter
or something like that as well as the factory. - We will need to rebase
master
and split the test case
4c91c10
to
76f132b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one suggestion !
Also, please, put the static cast in CLIParser.hpp(230,54): warning C4244: '=': conversion from 'int' to 'uint16_t'
to prevent windows from complaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with green CI !
Gj @elianalf
… the example Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…for writer side filtering Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
…to allowing the writer side filtering Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
da99050
to
62dd37e
Compare
CI passed at the previous commit. The last commit only add the license comment. |
Signed-off-by: elianalf <62831776+elianalf@users.noreply.github.com>
66de49c
to
4dac17a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ! Latest commit only adding licenses, ci was green
Description
This PR is part of a suite of PR which would make a refactor in the repository examples.
It is intended to apply to most of the examples, by making them homogeneous, more understandable, and more specific to the case they were meant to be.
In this content filter example, the main changes are:
Contributor Checklist
versions.md
file (if applicable).Reviewer Checklist