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

Preparing ex_doc to switch to EarmarkParser #1209

Merged
merged 3 commits into from
Jul 17, 2020

Conversation

RobertDober
Copy link
Contributor

@RobertDober RobertDober commented Jun 30, 2020

Well maybe this is a little bit early, Earmark will remain at your disposal if you prefer

If you want to wait until Earmark uses EarmarkParser for some versions or not switch at all I will not be offended (well not much)

However ultimately I'd like to use ex_doc as an indirect dependency in Earmark so if you want to switch one day that would be great.

This PR just gives you the option and I'll try to sync it with upstream as much as I can

Copy link
Member

@wojtekmach wojtekmach left a comment

Choose a reason for hiding this comment

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

LGTM!

Since we are not using many features from Earmark, depending on just the parser piece is definitely a win. I'm a little bit concerned that the split might generate more work for you (people will report issues in the "wrong" repo, syncing releases, etc) but hopefully these would be just growing pains and things will stabilise over long-term.

lib/ex_doc/markdown/earmark.ex Outdated Show resolved Hide resolved
mix.lock Outdated Show resolved Hide resolved
@RobertDober
Copy link
Contributor Author

The split is risky and I might regret it, but I am a perfectionist and a risk taker.
I do not consider these traits being a quality but sometimes I just cannot hold back. (as they say above the Oracle of Delphi)

Ok enough psychoanalysis of an egocentric developer 😓 just as you might have noticed when upgrading Earmark beyond 1.4.7 you will use EarmarkParser anyway.

@RobertDober
Copy link
Contributor Author

LGTM!

Since we are not using many features from Earmark, depending on just the parser piece is definitely a win. I'm a little bit concerned that the split might generate more work for you (people will report issues in the "wrong" repo, syncing releases, etc) but hopefully these would be just growing pains and things will stabilise over long-term.

Definitely it will remain a problem but I am prepared to deal with it. The release procedure will take more time too as the bulk of changes will be in the parser, I am just a mad man I guess ...

@wojtekmach wojtekmach mentioned this pull request Jul 4, 2020
@wojtekmach wojtekmach merged commit 021c772 into elixir-lang:master Jul 17, 2020
@eksperimental
Copy link
Contributor

👏

@RobertDober
Copy link
Contributor Author

RobertDober commented Jul 17, 2020

This is really great for my dev circle. Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants