Skip to content

Conversation

@buuhuu
Copy link
Contributor

@buuhuu buuhuu commented Jun 28, 2021

This change removes methods/fields marked deprecated from all Java classes.

Related Issue

CIF-2137

Motivation and Context

Major release of the CIF Core Components.

Types of changes

  • 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:

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

@buuhuu buuhuu changed the title CIF-2037: remove deprecated methods from models CIF-2137: remove deprecated methods from models Jun 29, 2021
@codecov
Copy link

codecov bot commented Jun 29, 2021

Codecov Report

Merging #603 (2fee10e) into master (8818565) will increase coverage by 0.09%.
The diff coverage is 87.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #603      +/-   ##
============================================
+ Coverage     87.97%   88.06%   +0.09%     
+ Complexity     1396     1382      -14     
============================================
  Files           254      254              
  Lines          6472     6429      -43     
  Branches        969      964       -5     
============================================
- Hits           5694     5662      -32     
+ Misses          583      568      -15     
- Partials        195      199       +4     
Flag Coverage Δ
integration 64.22% <45.45%> (+0.82%) ⬆️
jest 85.34% <ø> (ø)
karma 94.83% <ø> (ø)
unittests 87.41% <86.36%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../internal/models/v1/breadcrumb/BreadcrumbImpl.java 88.88% <ø> (-0.23%) ⬇️
...mponents/internal/models/v1/button/ButtonImpl.java 51.06% <ø> (-1.02%) ⬇️
...dels/v1/categorylist/FeaturedCategoryListImpl.java 88.15% <ø> (-0.16%) ⬇️
...riencefragment/CommerceExperienceFragmentImpl.java 75.00% <ø> (-0.19%) ⬇️
...onents/internal/models/v1/product/ProductImpl.java 88.81% <0.00%> (-1.67%) ⬇️
...onents/internal/models/v1/product/VariantImpl.java 100.00% <ø> (+16.66%) ⬆️
...models/v1/productcarousel/ProductCarouselImpl.java 85.93% <ø> (-0.22%) ⬇️
...nternal/models/v1/productlist/ProductListImpl.java 94.73% <ø> (-0.07%) ⬇️
...models/v1/relatedproducts/RelatedProductsImpl.java 88.23% <ø> (-0.23%) ⬇️
...orefrontcontext/CategoryStorefrontContextImpl.java 100.00% <ø> (ø)
... and 18 more

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 8818565...2fee10e. Read the comment docs.

@buuhuu buuhuu marked this pull request as ready for review June 29, 2021 14:08
@buuhuu buuhuu added the enhancement New feature or request label Jun 30, 2021
/**
* @deprecated use sling models in all cases instead
*/
@Deprecated
Copy link
Collaborator

@LSantha LSantha Jun 30, 2021

Choose a reason for hiding this comment

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

If this PR is about removing deprecated features, I wonder if there is any chance to get rid of this constructor? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be great, yes. But that would require touching a lot of unit tests. I would prefer to keep it. It is now in a private package and removing it will not cause a breaking change.

We can create a follow up story to remove it and fix the unit tests.

@mhaack mhaack merged commit 7772e7c into master Jun 30, 2021
@mhaack mhaack deleted the issue/CIF-2137 branch June 30, 2021 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants