Skip to content

Conversation

@Rezix93
Copy link

@Rezix93 Rezix93 commented Jul 31, 2024

No description provided.

@arfio arfio self-requested a review July 31, 2024 20:55
@Rezix93 Rezix93 force-pushed the counterAspectChanges branch from 3a14055 to e444a3c Compare July 31, 2024 21:00
* @since 3.0
*/
protected void handleGroupedCounterAspect(ITmfEvent event, ITmfStateSystemBuilder ss, CounterAspect aspect, int rootQuark) {
protected void handleGroupedCounterAspect(ITmfEvent event, ITmfStateSystemBuilder ss, ITmfCounterAspect aspect, int rootQuark) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is detected as major API change. I think you have to keep the old signature and deprecated it, and have it call the new signature, casting the aspect to avoid recursion.

With this change, it will only require a minor version bump to 2.3.0, this must be done in the plug-in MANIFEST.MF and will affect all updated @SInCE tags in this commit, to be set to 2.3.

* Get the groups
*
* @return the groups
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that it overrides, you can remove the Javadoc here, the one from interface will be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

*
* @return the type of this counter
* @since 2.1
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that it overrides, you can remove the Javadoc here, the one from interface will be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the Javadoc of getType() that can be removed.

* @return the groups
* @since 2.2
*/
default Class<? extends ITmfEventAspect<?>>[] getGroups(){
Copy link
Contributor

Choose a reason for hiding this comment

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

space before {

Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

@Rezix93 Rezix93 force-pushed the counterAspectChanges branch 2 times, most recently from 0a8a421 to 7ac254b Compare August 2, 2024 13:59
* @return the groups
* @since 2.2
*/
default Class<? extends ITmfEventAspect<?>>[] getGroups(){
Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

* Get the groups
*
* @return the groups
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

*
* @return the type of this counter
* @since 2.1
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

not fixed

* Key to a relative root of the state system
* @param aspect
* Grouped counter aspect
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

API change requires bump to 2.3.0 in MANIFEST.MF and since tag changed to 2.3 here.

* Get the groups
*
* @return the groups
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

API change requires bump to 2.3.0 in MANIFEST.MF and since tag changed to 2.3 here.

Copy link
Contributor

Choose a reason for hiding this comment

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

2.3

* Gets the type of this counter
*
* @return the type of this counter
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

API change requires bump to 2.3.0 in MANIFEST.MF and since tag changed to 2.3 here.

Copy link
Contributor

Choose a reason for hiding this comment

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

2.3

Copy link
Contributor

Choose a reason for hiding this comment

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

also missing a @return tag

@Rezix93 Rezix93 force-pushed the counterAspectChanges branch 2 times, most recently from 87a6529 to 1482a21 Compare August 2, 2024 16:40
* {@inheritDoc}
*
* This is a conservative equals. It only works on very identical aspects.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one should stay, it extends the inherited Javadoc.

*
* @return the type of this counter
* @since 2.1
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the Javadoc of getType() that can be removed.

* Get the groups
*
* @return the groups
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

2.3

* Gets the type of this counter
*
* @return the type of this counter
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

2.3

* Gets the type of this counter
*
* @return the type of this counter
* @since 2.2
Copy link
Contributor

Choose a reason for hiding this comment

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

also missing a @return tag

@Rezix93 Rezix93 force-pushed the counterAspectChanges branch from 1482a21 to 7a2e4f0 Compare August 6, 2024 16:26
[Changed] Allowed ITmfCounterAspect populate counterAnalysis

Signed-off-by: Reza Rouhghalandari <reza.rouhghalandari@ericsson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants