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-918: remove unused PageContext #280

Merged
merged 4 commits into from
May 28, 2020
Merged

CIF-918: remove unused PageContext #280

merged 4 commits into from
May 28, 2020

Conversation

laurentiumagureanu
Copy link
Collaborator

Description

PageContext is not used anymore as it's functionality was replaced in React Components

Related Issue

CIF-918

How Has This Been Tested?

Manually

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.

Fix safe loading for window.CIF in CommerceGraphqlApi
@LSantha
Copy link
Collaborator

LSantha commented May 21, 2020

@laurentiumagureanu , could you please check if we still need the PageContext mock at https://github.com/adobe/aem-core-cif-components/blob/master/ui.apps/test/components/commerce/navigation/navigationTest.js#L25 ?
We probably don't need it and it can be removed.

@laurentiumagureanu
Copy link
Collaborator Author

@laurentiumagureanu , could you please check if we still need the PageContext mock at https://github.com/adobe/aem-core-cif-components/blob/master/ui.apps/test/components/commerce/navigation/navigationTest.js#L25 ?
We probably don't need it and it can be removed.

Yeah, I don't know how I missed that one, I searched for all references of PageContext.
I removed it now.

@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #280 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #280   +/-   ##
=========================================
  Coverage     63.58%   63.59%           
  Complexity      757      757           
=========================================
  Files           175      175           
  Lines          5440     5441    +1     
  Branches        853      853           
=========================================
+ Hits           3459     3460    +1     
  Misses         1864     1864           
  Partials        117      117           
Flag Coverage Δ Complexity Δ
#jest 42.13% <ø> (ø) 0.00 <ø> (ø)
#karma 94.89% <100.00%> (+0.01%) 0.00 <0.00> (ø)
#unittests 85.16% <ø> (ø) 757.00 <ø> (ø)
Impacted Files Coverage Δ Complexity Δ
...ore/cif/clientlibs/common/js/CommerceGraphqlApi.js 85.71% <100.00%> (+0.23%) 0.00 <0.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 e8d1e54...ecd3915. Read the comment docs.

@hbacila hbacila merged commit 9ec7482 into master May 28, 2020
@hbacila hbacila deleted the issues/CIF-918 branch May 28, 2020 15:04
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.

4 participants