feat: change to markdown and map html to markdown #688

whilefoo · 2023-08-29T12:46:22Z

Resolves #673

netlify · 2023-08-29T12:46:27Z

✅ Deploy Preview for ubiquibot-staging ready!

Name	Link
🔨 Latest commit	`8e6ddaa`
🔍 Latest deploy log	https://app.netlify.com/sites/ubiquibot-staging/deploys/651db82cce1e250008a3ca1b
😎 Deploy Preview	https://deploy-preview-688--ubiquibot-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

whilefoo · 2023-08-29T12:50:03Z

src/helpers/comment.ts

+  // dynamic import of mdast
+  const { fromMarkdown } = await import("mdast-util-from-markdown");
+  const { gfmFromMarkdown } = await import("mdast-util-gfm");
+  const { gfm } = await import("micromark-extension-gfm");


I tried importing normally at the top but I got the error:

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/whilefoo/ubiquity/ubiquibot/node_modules/mdast-util-from-markdown/index.js from /Users/whilefoo/ubiquity/ubiquibot/lib/src/helpers/comment.js not supported. Instead change the require of index.js in /Users/whilefoo/ubiquity/ubiquibot/lib/src/helpers/comment.js to a dynamic import() which is available in all CommonJS modules.

After that I changed to dynamic import in the function but the error stays the same. I checked the compiled javascript and it's still require() - it seems every import is compiled to require.

@rndquu any ideas?

I guess this is a related issue #378

And this is a related PR #377 (which had been reverted because we got rid of mdast packages)

it seems #377 was supposed to fix the issue but it doesn't - instead of throwing the error at startup, it throws the error when the function runs.

So it throws the same error both when modules are imported at the top of the comment.ts file and in the parseComments() function, right?

So it throws the same error both when modules are imported at the top of the comment.ts file and in the parseComments() function, right?

yes

I tried changing Typescript settings to transpile to ESM and added "type": "module" to package.json, but I get a different error because Probot is using require() internally

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/whilefoo/ubiquity/ubiquibot/lib/src/index.js from /Users/whilefoo/ubiquity/ubiquibot/node_modules/probot/lib/helpers/resolve-app-function.js not supported. Instead change the require of index.js in /Users/whilefoo/ubiquity/ubiquibot/node_modules/probot/lib/helpers/resolve-app-function.js to a dynamic import() which is available in all CommonJS modules.

node_modules/probot/lib/helpers/resolve-app-function.js is using require()

Perhaps we could downgrade all 3 packages (mdast-util-from-markdown, mdast-util-gfm, micromark-extension-gfm) to their "pre ESM" versions

I tried downgrading and I have an issue with mdast-util-gfm because that version didn't have any types so typescript is complaining. I will see if there's anything I can do

https://github.com/ubiquity/ubiquibot/blob/incentive-based-on-comment/src/helpers/comment.ts#L23

Please take a look at the incentive-based-on-comment branch @whilefoo. It has been done with mdast library.
The blocker on that branch was the function timeout: 10s as of now.

I hope you can get reusable/helpful methods from that branch.

Please take a look at the incentive-based-on-comment branch @whilefoo.

That is exactly how I've done it but it's not working, you can check the whole conversation for context

whilefoo · 2023-09-03T14:13:22Z

src/handlers/payout/post.ts

+  [MarkdownItem.LinkReference]: HTMLItem.A,
+  [MarkdownItem.ImageReference]: HTMLItem.IMG,
+  [MarkdownItem.FootnoteReference]: HTMLItem.SUP,
+  [MarkdownItem.FootnoteDefinition]: HTMLItem.P,


I'm not sure how can we represent footnote reference and definition by HTML? @pavlovcik

Example of footnote¹

Footnotes

Footnote 1 ↩

Very cool. I didn't know about this but I guess we can just file an issue and kick the can down the road?

whilefoo · 2023-09-03T14:14:18Z

src/handlers/payout/post.ts

+  [MarkdownItem.Link]: HTMLItem.A,
+  [MarkdownItem.Image]: HTMLItem.IMG,
+  [MarkdownItem.BlockQuote]: HTMLItem.BLOCKQUOTE,
+  [MarkdownItem.Code]: HTMLItem.PRE,


I'm not sure if it's gonna be clear to partners that <pre> refers to

code

and <code> refers to inline code

whilefoo · 2023-09-03T14:17:17Z

@pavlovcik can you clarify if task creator and task assignee are eligible for task conversation rewards? I'd guess not because task creator will make comments to clarify his task and task assignee will comment to explain his solution or ask questions to the task creator

rndquu · 2023-09-04T07:19:47Z

@pavlovcik can you clarify if task creator and task assignee are eligible for task conversation rewards? I'd guess not because task creator will make comments to clarify his task and task assignee will comment to explain his solution or ask questions to the task creator

Oh, I actually thought that task creator is also eligible for task conversation rewards. Ok then, I will update the description for #704

0x4007 · 2023-09-04T07:40:13Z

@pavlovcik can you clarify if task creator and task assignee are eligible for task conversation rewards? I'd guess not because task creator will make comments to clarify his task and task assignee will comment to explain his solution or ask questions to the task creator

That's a good observation. Here are all the roles that I see and if they should be eligible for comment incentives. What if we make them configurable?

Something like:

### on issue view

- assignee: false
- issuer: false
// - reviewer: n/a
- default: true

### on pull request view 

- assignee: false
- issuer: true
- reviewer: true
- default: true

0x4007 · 2023-09-26T06:17:59Z

Updated roles:

Issue

assignee: false
issuer: false
// - collaborator: n/a
contributor: true

Review

assignee: false
issuer: true
collaborator: true
contributor: true

Debating on if we should make a "reviewer" role which kicks in if you helped with the review. These are who need to be highly incentivized because review seems to be very burdensome.

feat: change to markdown and map html to markdown

09485ef

whilefoo commented Aug 29, 2023

View reviewed changes

whilefoo added 3 commits August 31, 2023 15:27

fix: use old version of mdast libraries that can be used with require

b5446a8

fix: markdown types

d00af2c

feat: html tags enum

0b5e149

whilefoo commented Sep 3, 2023

View reviewed changes

This was referenced Sep 6, 2023

Default config and config change #668

Closed

Update post.ts #674

Closed

0x4007 mentioned this pull request Sep 14, 2023

Comment Incentive Config: Footnote reference and definition #766

Open

whilefoo added 2 commits September 19, 2023 22:30

Merge remote-tracking branch 'upstream/development' into incentives-fix

9e88f65

feat: add config

ed023ba

Merge remote-tracking branch 'upstream/development' into incentives-fix

8e6ddaa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: change to markdown and map html to markdown #688

feat: change to markdown and map html to markdown #688

whilefoo commented Aug 29, 2023 •

edited

netlify bot commented Aug 29, 2023 •

edited

whilefoo Aug 29, 2023

rndquu Aug 29, 2023

whilefoo Aug 29, 2023

rndquu Aug 29, 2023

whilefoo Aug 29, 2023

whilefoo Aug 29, 2023

rndquu Aug 29, 2023

whilefoo Aug 29, 2023

0xcodercrane Aug 30, 2023

whilefoo Aug 30, 2023

whilefoo Sep 3, 2023

0x4007 Sep 14, 2023

whilefoo Sep 3, 2023

whilefoo commented Sep 3, 2023

rndquu commented Sep 4, 2023

0x4007 commented Sep 4, 2023 •

edited

0x4007 commented Sep 26, 2023 •

edited

feat: change to markdown and map html to markdown #688

Are you sure you want to change the base?

feat: change to markdown and map html to markdown #688

Conversation

whilefoo commented Aug 29, 2023 • edited

netlify bot commented Aug 29, 2023 • edited

✅ Deploy Preview for ubiquibot-staging ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Footnotes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whilefoo commented Sep 3, 2023

rndquu commented Sep 4, 2023

0x4007 commented Sep 4, 2023 • edited

0x4007 commented Sep 26, 2023 • edited

Issue

Review

whilefoo commented Aug 29, 2023 •

edited

netlify bot commented Aug 29, 2023 •

edited

0x4007 commented Sep 4, 2023 •

edited

0x4007 commented Sep 26, 2023 •

edited