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

CIF-1220: Fix CQBP-84 violation for Venia Reference #261

Merged
merged 2 commits into from
May 6, 2020
Merged

Conversation

hbacila
Copy link
Contributor

@hbacila hbacila commented May 5, 2020

Description

Remove @ProviderType annotation if the interface is implemented / extended by custom code.
This fixes CM code quality reported issue:

../commerceinternalstageprogram/core/models/commerce/MyProductTeaser.java: The product interface com.adobe.cq.commerce.core.components.models.productteaser.ProductTeaser annotated with @ProviderType should not be implemented by custom code.

Related Issue

https://jira.corp.adobe.com/browse/CIF-1220

Motivation and Context

Custom component models should not implement / extend core component interfaces annotated with @ProviderType to avoid breaking changes in core interface.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [x ] I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • [x ] I have read the CONTRIBUTING document.
  • [x ] I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • [x ] I ran all tests locally and they pass.

- remove @ProviderType annotation if the interface is implemented / extended by custom code
Copy link
Contributor

@cjelger cjelger left a comment

Choose a reason for hiding this comment

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

The @ProviderType is actually set on all the Sling models, shouldn't we remove this in all the models? Customers might extend other components, the product teaser is just an example. We should check how this is done in the WCM core components, I checked a few and they had either @ConsumerType or nothing (which defaults to consumer type). WDYT?

@hbacila
Copy link
Contributor Author

hbacila commented May 5, 2020

I think we should remove @ProviderType for all models unless we decide which ones should be "final" (if any). Indeed the WCM core components models are all @ConsumerType (explicit or implicit)

@mhaack
Copy link
Contributor

mhaack commented May 5, 2020

+1 for @ConsumerType and align with WCM core components

- remove @ProviderType annotation from all component models
@hbacila
Copy link
Contributor Author

hbacila commented May 5, 2020

Removed @ProviderType annotations from all component models.

@codecov
Copy link

codecov bot commented May 5, 2020

Codecov Report

Merging #261 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #261   +/-   ##
=========================================
  Coverage     62.32%   62.32%           
  Complexity      728      728           
=========================================
  Files           171      171           
  Lines          5240     5240           
  Branches        820      820           
=========================================
  Hits           3266     3266           
  Misses         1862     1862           
  Partials        112      112           
Flag Coverage Δ Complexity Δ
#jest 40.21% <ø> (ø) 0.00 <ø> (ø)
#karma 94.88% <ø> (ø) 0.00 <ø> (ø)
#unittests 84.82% <ø> (ø) 728.00 <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c6604c...5b8fd3b. Read the comment docs.

@hbacila hbacila merged commit dc6504f into master May 6, 2020
@hbacila hbacila deleted the issues/CIF-1220 branch May 6, 2020 09:54
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.

3 participants