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

fix: change Paragon to a peerDependency #425

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MaxFrank13
Copy link
Member

When carrying over some changes from the OpenedX version of the footer, we changed Paragon from a peerDependency to a regular dependency (source). The Open edX footer has since fixed this and correctly has Paragon set to a peerDependency (source).

Paragon should be a peerDependency here so that users don't end up having to download multiple versions of Paragon and we don't introduce any style regressions.

"js-cookie": "^3.0.1",
"lodash": "^4.17.21"
},
"peerDependencies": {
"@edx/frontend-platform": "^7.0.0 || ^8.0.0",
"@openedx/paragon": ">= 21.11.3 < 23.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

[sanity check] is there anything in the major upgrade from v21 to v22 that could break frontend-component-footer-edx? That is, will supporting an older v21 version be compatible given it was previously using v22? If not, then I agree, expanding the support back to v21 is reasonable :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh good call out. I was just replicating what was done in the Open edX version. This at least gives some reassurance. But looking at the breaking changes for v22, I don't see anything in there that would cause footer to break. The components called out in the release notes aren't used by frontend-component-footer-edx.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for verifying!

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