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/TR-6195/remove tao-core-libs dependency #187

Merged
merged 10 commits into from
Aug 12, 2024

Conversation

oatymart
Copy link
Contributor

@oatymart oatymart commented Jul 15, 2024

https://oat-sa.atlassian.net/browse/TR-6195

The link between tao-core-sdk and tao-core-libs has a small surface area: only 2 libs files are really used in sdk.
This PR aims to break that link, meaning that tao-core-sdk can then be installed in projects (e.g. Nextgen) without installing tao-core-libs and all the old CurrentGen dependencies it brings in.

Dependencies removed because no usage found in tao-core-sdk: dompurify gamp interactjs mime moment-timezone popper.js promise-limit raphael select2 tooltip.js
Dependencies removed due to better browser support: fastestsmallesttextencoderdecoder webcrypto-shim

How to test

In CurrentGen TAO

Deployed to: https://test-tr-6195-authoring.v3.playground.taocloud.org/

  • Install tao-core from Fix/TR-6195/Upgrade tao-core-sdk tao-core#4077
  • Manually test TAO's main functionality. The module uuid is quite commonly called, in case you want to check it.
  • Try with DEBUG_MODE true and false in config/generis.config.php

In LDS

In a NextGen project (e.g. tao-deliver-fe)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines -27 to -28
import 'webcrypto-shim';
import { TextEncoder } from 'fastestsmallesttextencoderdecoder';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are out because

src/core/uuid.js Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

github-actions bot commented Aug 1, 2024

Coverage Report

Totals Coverage
Statements: 90.85% ( 2641 / 2907 )
Methods: 95.25% ( 601 / 631 )

@oatymart oatymart marked this pull request as ready for review August 1, 2024 16:13
@oatymart oatymart self-assigned this Aug 1, 2024
Copy link
Contributor

@olga-kulish olga-kulish left a comment

Choose a reason for hiding this comment

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

Very nice improvement! I'll approve when tao-deliver-fe issue (if actual) is resolved

Copy link

github-actions bot commented Aug 2, 2024

Version

Target Version 3.2.0
Last version 3.1.1

There are 0 BREAKING CHANGE, 1 feature, 1 fix

@olga-kulish olga-kulish self-requested a review August 5, 2024 07:08
Copy link
Contributor

@olga-kulish olga-kulish left a comment

Choose a reason for hiding this comment

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

Very nice improvement

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code is respecting code style rules
  • New code is respecting best practices
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful

Copy link
Contributor

@yairkukielka yairkukielka left a comment

Choose a reason for hiding this comment

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

Nice job! LGTM

Copy link
Contributor

@jsconan jsconan left a comment

Choose a reason for hiding this comment

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

Review Checklist

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code respects code style rules
  • New code respects best practices
  • New code is not subject to concurrency issues (if applicable)
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful
  • Pull request's target is not master
  • Commits are following conventional commits
  • Commits messages are meaningful
  • Commits are atomic

@olga-kulish olga-kulish self-assigned this Aug 12, 2024
@olga-kulish olga-kulish merged commit 97e4418 into develop Aug 12, 2024
3 checks passed
@olga-kulish olga-kulish deleted the fix/TR-6195/remove-tao-core-libs-dependency branch August 12, 2024 12:56
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