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

MM-56469 - change out library for converting html to markdown. #517

Merged
merged 3 commits into from
Mar 8, 2024

Conversation

sbishel
Copy link
Member

@sbishel sbishel commented Feb 28, 2024

Summary

Changes out the library used for converting HTML to Markdown. Also implements unit test for all the potential conversions we support...if any were missed, let's add them.

http://github.com/JohannesKaufmann/html-to-markdown

The prior library did not handle tables without a THEAD very well, tables create in MS Teams do not have this row. This library creates GibHub Flavored Markdown and is extensible via Plugins.

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-56469

@sbishel sbishel added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 3: Security Review Review requested from Security Team labels Feb 28, 2024
@sbishel
Copy link
Member Author

sbishel commented Feb 28, 2024

@esarafianou - Not sure if security wants to review now or with the plugin as a whole later....

@sbishel sbishel changed the title change out library for converting html to markdown. MM-56469 - change out library for converting html to markdown. Feb 28, 2024
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Wow, nice work @sbishel :)

@lieut-data lieut-data added this to the v1.10 milestone Mar 3, 2024
Copy link

@esarafianou esarafianou left a comment

Choose a reason for hiding this comment

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

LGTM.

cc. @jupenur let me know if you'd like to have an extra look at the new library.

@sbishel sbishel requested a review from lindy65 March 7, 2024 15:18
@sbishel sbishel removed 2: Dev Review Requires review by a core committer 3: Security Review Review requested from Security Team labels Mar 7, 2024
Copy link

@lindy65 lindy65 left a comment

Choose a reason for hiding this comment

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

Thanks @sbishel
QA-deferring to test as bug bash post merge

@lindy65 lindy65 added 4: Reviews Complete All reviewers have approved the pull request QA/Deferred Agreement with QA that these changes will be tested post-merge and removed 3: QA Review Requires review by a QA tester labels Mar 8, 2024
@lieut-data lieut-data merged commit 3e15489 into main Mar 8, 2024
7 checks passed
@lieut-data lieut-data deleted the MM-56469-Update-library branch March 8, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request QA/Deferred Agreement with QA that these changes will be tested post-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants