Rosey / markdown-draft-js

A tool to convert content created in DraftJS to markdown and vice versa.
https://rosey.github.io/markdown-draft-js/
MIT License
318 stars 69 forks source link

Converted #34 to pull request #46

Closed Tam closed 4 years ago

Tam commented 6 years ago

A straight copy-paste from #34

lefnire commented 6 years ago

@Tam I couldn't get your PR working for images. I capitalized the 'image' entities on a whim (since similar entities have that formatting) and it worked. That's to say, I don't know what I'm doing, so please check my work :)

https://github.com/lefnire/markdown-draft-js/commit/55ccad01166c2d31d002536e62a24a5744448fdc (if √ then please add to your PR)

Tam commented 6 years ago

@lefnire If it makes you feel better, I also have no idea what I'm doing 😄. I've added your changes to my PR.

alibosworth commented 6 years ago

I am not the author (of this repo or these changes) but I think that this would be much much nicer if it was broken up into multiple commits representing the distinct features/improvements.

As per the maintenance note Rosey is on maternity leave so I think breaking this up into pieces with well worded commits will make it easier for her to review and merge when she has a chance.

It is nice that you guys are working to bring in the changes sent in by @atanasster in https://github.com/Rosey/markdown-draft-js/issues/34 but I also wonder why they didn't open a PR themselves? They seem to have some sparse GH history so they seem git capable.

Also the tests are failing right now so at a minimum they need to be updated to pass. Indentation seems to be messed up as well (see logs)

lefnire commented 6 years ago

Definitely agree on multiple PRs; more likely to get faster reviews/merges.

It is nice that you guys are working to bring in the changes sent in by @atanasster in #34 but I also wonder why they didn't open a PR themselves? They seem to have some sparse GH history so they seem git capable.

"Don't build anything someone else said", right? Know how many Github comments are fire-and-forget? These are two positive contributions: (A) I have some ideas, (B) great, I'll build those; what's the problem?

atanasster commented 6 years ago

Hi, thanks for adding the changes - I didnt know Rossey was on maternity leave and just wanted to make sure she is ok with the changes before submitting PRs. In the meantime I added several changes and fixes while using the module in production and would be happy to contribute them as well. I am really not that fluent with git but can collaborate on bringing the repo up to date

atanasster commented 6 years ago

I see now why I didn't know about Rossey's leave - I posted the note and also sent an email to Rossey in October 2017 (and had completely forgotten about this post since there never was any feedback or show of interest and so kept working on the lib on my end to make it fit my needs, assuming that the changes are not in the direction the original author wanted to lead the library), while the maternity leave note is from December 2017. Anyway, if anyone is interested in collaborating, pls let me know - would be happy to contribute back.

Rosey commented 6 years ago

Hello! I did see the original zip file but never got around to converting it because -

Anyway, it turns out that taking care of a newborn is a full time job so I still haven't any time to look at this in more detail, but I wanted to provide a bit of context for why this hadn't been merged or anything yet. If you guys have any thoughts on why I'm wrong on any of the points above (eg re: "how the library should be used") please share them! Or if there's something (like underline support) that you're really missing and want to include, feel free to split out into a smaller commit/PR with appropriate tests as well and I'll try to merge it :)

Thanks very much for all your interest and comments!