Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Conversation

@clero
Copy link
Contributor

@clero clero commented Mar 18, 2015

Depends on #60
Please review from a1a4dd3

This is a rework of Pfw criterion. Please do not merge now but some feedback on designs changes would be appreciate.

Next step:

  • Update coding style everywhere
  • Use Bit set for criterion state. (in an other pull request)

@clero clero force-pushed the criterion_rework branch 5 times, most recently from 02794f0 to 039564b Compare March 23, 2015 08:46
@clero clero force-pushed the criterion_rework branch from 039564b to ac63b8f Compare March 31, 2015 08:16
@clero clero force-pushed the criterion_rework branch 2 times, most recently from e84d8f4 to 9685b70 Compare April 13, 2015 16:55
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.21%) to 69.93% when pulling 9685b70 on clero:criterion_rework into 6d605e3 on 01org:master.

@clero clero force-pushed the criterion_rework branch 3 times, most recently from 8b0a698 to 5dbcc25 Compare April 14, 2015 09:14
@coveralls
Copy link

Coverage Status

Coverage increased (+0.37%) to 70.51% when pulling 5dbcc25 on clero:criterion_rework into 6d605e3 on 01org:master.

@clero clero force-pushed the criterion_rework branch 3 times, most recently from af728e0 to 43d8e90 Compare April 14, 2015 14:31
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 70.54% when pulling 43d8e90 on clero:criterion_rework into 6d605e3 on 01org:master.

clero added 13 commits April 14, 2015 17:22
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Some elements are logging but ParameterMgr is able
to do the same.
This patch lets the ParameterMgr handle logging
whenever it's possible.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
String lists are often used as containers for function results.
To avoid copy paste, this patch introduces the Results type
to replace std::list<std::string> when relevant.
Documentation has been updated on modified prototype.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
This log is meaningless, as we know subsystems we are
attempting to load. We log only if there is an error.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
This log information is easy to retrieve by looking
the structure file.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
The client may choose if he wants to log the error
when adding a criterion.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
In prevision of logging rework, this patch
removes logging from plugins.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
uint8_t type seems to be buggy when we try to log it through stream API.
This patch uses uint32_t type instead to count criterion modifications.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Logs were provided by Element god class. This patch
moves logging in the log library which is used by
the ParameterMgr.
Logs remains almost identical from before.
Context title after context closing bracket has been
removed to enhance readability.

Change-Id: Ic7dca65c3cc88eb06a1883c4fb01af9809b19e42
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Plugins are now able to log.
An ILogger object is provided to plugins to do that.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
clero and others added 3 commits April 14, 2015 17:22
Change-Id: Ib31547b3f899f96b837ac125ea7b0feb8c946760
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: David Wagner <david.wagner@intel.com>
@dawagner
Copy link
Contributor

please rebase onto and resubmit against "next"

@clero clero force-pushed the criterion_rework branch from 43d8e90 to 680eb40 Compare April 14, 2015 15:49
dawagner and others added 5 commits April 14, 2015 19:14
init function is defined in Element god class
but is used only for plugins initialization
after structure and settings loading.
This patch delegate Subsystem initialization
to SystemClass which is the direct parent.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
The element class really looks like a god class with stuff that has
really nothing to do with its role. toString and appendTitle are good
example of stuff that should be move away from it.

This patch move those two functions into utility where they seems to
belong.

Signed-off-by: Sébastien Gonzalve <oznog@users.noreply.github.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
SelectionCriteria class is dependant from Element class because
of the checksum calculation algorithm which is based on child
name.
In preparation of a rework, we exclude it from the structure
checksum calculation. Moreover the criterion mechanism is not
a part of the parameter-framework structure. Thus, this decision
is fair.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
ParameterManager does not have to be a part of the Element tree.
Indeed, it is not represented in any xml file.
This patch avoids this inheritance and removes some "tricks" which
was used to forget the ParameterMgr root element.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
@clero clero force-pushed the criterion_rework branch from 680eb40 to 06f8027 Compare April 15, 2015 06:59
clero added 15 commits April 15, 2015 08:59
C++11 is now available on Android, thus we can start
to use it in the parameter-framework.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
This type is not a part of the parameter-framework. It's seems
it has been abandoned, all remaining traces are now deleted.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
SelectionCriteria object knows at compile time the type of his
children. Thus the Element inheritance is not necessary.
This patch starts the rework of the criterion subtree.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Once again, Element class is used to hide aggregation link.
This patch uses a list instead of the child link.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Once again, Element class is used to hide aggregation link.
This patch uses a map to store criteria instances.
It also avoids static casting all criteria.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Criteria are now using Element as base class only for "leaf"
element SelectionCriterion and SelectionCriterionType.
This branch of parameter-tree is now completely removed.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
This delegate was used as factory in order to create types.
It was only a list wrapper, thus it can be removed.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
This class is a list wrapper which exposes the same API
than the class it compose.
Moreover, this internal object is often returned to
external object. This patch removes this delegate and let
external classes deal with SelectionCriteria class.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Criterion type (Inclusive or Exclusive) was determined
thanks to Strategy pattern through SelectionCriterionType
class.
It leads to an overhead for the client who need to create
the criterion type and then the criterion itself. The client
can also delete the criterion type which can lead to hard to
debug bugs.
This patch replaces this pattern by inheritance. We now have
two criterion types, SelectionCriterion which is the basic
Exclusive criterion and Inclusive whose specialize it..

This patch has some client API impact:
    - ISelectionCriterionTypeInterface has been merged with
      ISelectionCriterionInterface
    - isTypeInclusive has been renamed isInclusive
    - getFormattedState does not take any argument any more

Signed-off-by: Jules Clero <julesx.clero@intel.com>
SelectionCriterion class was defining includes, excludes, is and isNot
state matching method. Nevertheless, includes and excludes ones has
meaning only for InclusiveCriterion.
The SelectionCriterionRule class was checking before matching if the
method was authorized for the criterion.
This patch introduces MatchMethod internal SelectionCriterion type
which defines methods which can be used. Thus includes and excludes
ones are defined only in InclusiveCriterion class where they belongs.

Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
@clero
Copy link
Contributor Author

clero commented Apr 15, 2015

replaced by #96

@clero clero closed this Apr 15, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+0.48%) to 70.61% when pulling 06f8027 on clero:criterion_rework into 541735d on 01org:master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants