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

Major Refactor: update everything #10

Merged
merged 15 commits into from
Jul 31, 2019
Merged

Major Refactor: update everything #10

merged 15 commits into from
Jul 31, 2019

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Jul 8, 2019

I have refactored the code to upgrade all dependencies to the latest and better match marked's style.

Changes

  • Update deps to latest
  • Make async to work with parse5 streams
  • Remove unneeded dependencies
  • use same eslint config as marked tests
  • Remove ignoreDuplicateAttributes option since parse5 doesn't support duplicate attributes any more (see inikulin/parse5@225d27f)
  • replace istanbul with nyc (active version of istanbul)

Fixes #5
Fixes #6

@UziTech UziTech changed the title Major Refactor: update everything [WIP] Major Refactor: update everything Jul 8, 2019
@UziTech
Copy link
Member Author

UziTech commented Jul 8, 2019

I'm working on testing marked with this version to make sure it still works

@UziTech
Copy link
Member Author

UziTech commented Jul 8, 2019

Bad news: Parse5 does not support node v4 so marked's tests in node v4 would break but all other tests work.

should we wait on this or stop testing marked in node v4? we already quit testing in node v0.10 even though we still support it because of the testing framework (markedjs/marked#1366)

/cc @styfle @joshbruce @davisjam

@styfle
Copy link
Member

styfle commented Jul 8, 2019

quit testing in node v0.10 even though we still support it

What do you mean we still support it? I specifically remember dropping support in marked 0.6.0.

@UziTech
Copy link
Member Author

UziTech commented Jul 9, 2019

@styfle You're right. I was thinking of how we stopped testing node v0.10 in marked v0.5.2 but still supported it until v0.6.0

We could do the same with node v4. stop testing it but still support it.

@styfle
Copy link
Member

styfle commented Jul 9, 2019

It seems like it would be easy to accidentally introduce a breaking change if CI is not testing the environment we expect to support.

@UziTech
Copy link
Member Author

UziTech commented Jul 9, 2019

We could still support it as in if someone brings up an issue we will fix it.

If someone is still on node 4 chances are pretty good that they are on an old version of marked also.

@UziTech
Copy link
Member Author

UziTech commented Jul 9, 2019

or maybe we can still run a simple test on node v4 that would just make sure we don't introduce syntax that that doesn't work in node v4.

@styfle
Copy link
Member

styfle commented Jul 9, 2019

I see, those are both good options.

If I recall, its not the Node support that was holding back marked but the Browser support.

It was a coincidence that Node and IE had (almost) feature parity at one point.

@UziTech
Copy link
Member Author

UziTech commented Jul 31, 2019

I think eslint with ecmaVersion: 5 should be enough to keep things working for older browsers.

Once this is merged I will work on updating marked to use this promisified version

@UziTech UziTech changed the title [WIP] Major Refactor: update everything Major Refactor: update everything Jul 31, 2019
@UziTech UziTech merged commit fd27b3a into markedjs:master Jul 31, 2019
@UziTech UziTech deleted the update-diff branch July 31, 2019 16:33
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.

update diff update parse5
2 participants