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

Remove cobertura-parse (reverts #243) #257

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Remove cobertura-parse (reverts #243) #257

merged 1 commit into from
Nov 26, 2019

Conversation

XhmikosR
Copy link
Contributor

cobertura-parse is archived and in bad shape. They even have mocha in dependencies.

See also my comment

@ly-cultureiq
Copy link
Contributor

mocha is only used for tests it seems - not for actual code. so it should just go into devdependencies. otherwise the cobertura spec hasnt changed it seems - so the code works perfectly fine

@XhmikosR
Copy link
Contributor Author

You are missing the point. cobertura-parse has mocha in its dependencies, thus everyone who depends on it has to download and install the useless mocha dep.

@XhmikosR
Copy link
Contributor Author

Or to put it better: everyone who depends on cobertura-parse, downloads and installs the useless mocha dependency inadvertently.

@nickmerwin nickmerwin changed the title Revert #243. Remove cobertura-parse (reverts #243) Nov 26, 2019
cobertura-parse is archived and in bad shape. They even have mocha in dependencies.
@nickmerwin
Copy link
Owner

@ly-cultureiq apologies for the back and forth, I didn't realize the state of the https://github.com/vokal/cobertura-parse package. If there was an actively maintained alternative I'd be happy to bring it back in, but for now I'd prefer not to have this package depend on an archived one.

@nickmerwin nickmerwin merged commit 1bceeff into nickmerwin:master Nov 26, 2019
@XhmikosR XhmikosR deleted the revert-cob branch November 26, 2019 17:32
@nickmerwin
Copy link
Owner

@XhmikosR thank you for the rebase!

@github-actions
Copy link

Coverage Status

Coverage decreased (-0.02%) to 97.118% when pulling 4bb580f on XhmikosR:revert-cob into 4aa11a2 on nickmerwin:master.

@ly-cultureiq
Copy link
Contributor

@nickmerwin no problems - im now noticing https://github.com/coverage-report/cobertura-json - which seems to be more maintained and should be a drop in replacement - Im happy to make a pr based of that if you and @XhmikosR think that would work

@XhmikosR
Copy link
Contributor Author

That seems better but keep in mind https://packagephobia.now.sh/result?p=%40cvrg-report%2Fcobertura-json

@ly-cultureiq
Copy link
Contributor

i saw that - its all direct dependencies of xml2js - so theres not really a way around that install size...

@XhmikosR
Copy link
Contributor Author

It's @nickmerwin to decide. xml2js could be improved in the future but the main issue here IMHO is that https://github.com/coverage-report/cobertura-json is a fork.

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.

3 participants