Skip to content

Conversation

@chrisguttandin
Copy link
Contributor

This pull request updates assetgraph to version 5. I did this because I realized that the version of acorn that assetgraph at version 4 uses can't parse modern JavaScript syntax like the spread operator.

I order to get hyperlink working with assetgraph v5, I had to remove the compatibility check. It used some private methods of assetgraph which are not in place anymore after the update.

Please let me know if you have an idea how I could reintroduce the compatibility check or if there is anything else I need to do to make this pull request mergeable.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.6%) to 75.362% when pulling 6d10c78 on chrisguttandin:master into 695abb5 on Munter:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.6%) to 75.362% when pulling 6d10c78 on chrisguttandin:master into 695abb5 on Munter:master.

@papandreou
Copy link
Collaborator

Hi Chris,

Thanks for looking into this! I meant to try to tackle the assetgraph 5 upgrade as part of #143, but never really got around to it. I agree that updating outside of that PR would be worth it for the new features alone now that we got that JS parser upgrade in.

I'll look into it as soon as I get the chance, will try to lend a hand with the compatibility checks.

@chrisguttandin
Copy link
Contributor Author

Alright, please let me know if there is anything I can do.

@papandreou papandreou merged commit 13e5f47 into Munter:master Apr 4, 2019
@papandreou
Copy link
Collaborator

@chrisguttandin, I found a way to reintroduce the compatibility check, fixed for next release. Thanks again for the contribution! 🎉

@Munter, can you help with getting a new version published?

@Munter
Copy link
Owner

Munter commented Apr 6, 2019

@papandreou Great job on adding back the compatibility check and at the same time reducing its annoyance level!

Thanks for the contribution @chrisguttandin

I've released https://github.com/Munter/hyperlink/releases/tag/v4.1.0

@chrisguttandin
Copy link
Contributor Author

Many thanks for the quick response @papandreou and @Munter.

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.

4 participants