search

openedx / frontend-component-footer

Site footer component for edX frontend apps.
GNU Affero General Public License v3.0
6 stars 97 forks source link

feat: upgrade frontend platform to version 3 #241

Closed leangseu-edx closed 2 years ago

leangseu-edx commented 2 years ago

BREAKING CHANGE:

codecov[bot] commented 2 years ago

Codecov Report

Base: 81.25% // Head: 81.25% // No change to project coverage :thumbsup:

Coverage data is based on head (ec60b0c) compared to base (b06c8b5). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #241 +/- ## ======================================= Coverage 81.25% 81.25% ======================================= Files 4 4 Lines 32 32 Branches 4 4 ======================================= Hits 26 26 Misses 6 6 ``` Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

leangseu-edx commented 2 years ago

Update requested change

adamstankiewicz commented 2 years ago

@arbrandes @leangseu-edx Just a heads up, this PR could have been released as a non-breaking change if the peer dependency range denoted compatibility with v2 and v3 of @edx/frontend-platform (e.g., ^2.0.0 || ^3.0.0), which is true since this component doesn't use caching explicitly as @arbrandes identified earlier 😄

@leangseu-edx Also, reminder... if we make a change in this repository that is a breaking change for the component API or peer dependencies, an equivalent change should be made in our -footer-edx equivalent repository.

adamstankiewicz commented 2 years ago

@leangseu-edx @arbrandes Also note, the PR description denoted this as a breaking change (which it was) but it wasn't actually released to NPM as a breaking change (only a new minor version was bumped)...

arbrandes commented 2 years ago

@adamstankiewicz, any idea what the commit message missed? I was under the impression only a BREAKING CHANGE: in the body would be enough. Do we normally also need an exclamation point after the tag? (feat!:)?

adamstankiewicz commented 2 years ago

@arbrandes BREAKING CHANGE: in the body should be all it requires, however, I don't think the commit that merged with master included the BREAKING CHANGE: in the commit body (it's only in the PR description).