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

Integrate churn Gem into Rubycritic to Speed Up RubyCritic Churn Computation #498

Open
hemalvarambhia opened this issue Dec 2, 2024 · 10 comments

Comments

@hemalvarambhia
Copy link

hemalvarambhia commented Dec 2, 2024

Problem: I use rubycritic on some repositories and some take 15+ minutes, which seems similar to #490.

Solution: I @Greg-Myers-SB found this gem by Dan Mayer (see Churn's Github page) and found it was a lot faster. Can using the said gem happen?

@hemalvarambhia hemalvarambhia changed the title Integrate churn gem into Rubycritic to speed up RubyCritic Churn Computation Integrate churn Gem into Rubycritic to Speed Up RubyCritic Churn Computation Dec 2, 2024
@Greg-Myers-SB
Copy link

On the same codebase.

Rubycritic's Churn:
image
image

Churn gem:
image
image

Churn gem is significantly faster, we might be able to make an adapter for other Churn sources and figure out what the major differences in results are.

@etagwerker
Copy link
Collaborator

Solution: I found this gem by Dan Mayer (see Churn's Github page) and found it was a lot faster. Can using the said gem happen?

Yes, I'm all for that. As a co-maintainer, if we can integrate with churn with minimal impact to current users I'm all for it. Thoughts, @nunosilva800?

Churn gem is significantly faster, we might be able to make an adapter for other Churn sources and figure out what the major differences in results are.

I would be inclined to release v5.x for a change like this one. That way we can flag it to rubycritic users that there is a big change getting released.

Ideally we want to keep the rubycritic test suite as is when switching to churn -- I'm not totally sure that there are good enough tests for the churn behavior though.

@Greg-Myers-SB
Copy link

Greg-Myers-SB commented Dec 2, 2024

Yep, of course it's not just speed, the churn gem is not a 1-1 with the current implementation and we may spend some time into looking at what the differences are.
Edit: I saw on the linked issue that support for HG/Mercurial/SVN are a topic.

@hemalvarambhia
Copy link
Author

@Greg-Myers-SB I saw that it has a hg analyser module. I thought Hg and mercurial are the same. How wrong am I?

@hemalvarambhia
Copy link
Author

Ideally we want to keep the rubycritic test suite as is when switching to churn -- I'm not totally sure that there are good enough tests for the churn behavior though.

@etagwerker and what kind of good enough is that? Is this something you would like addressed?

Edit: I'd like to credit @Greg-Myers-SB for finding the gem.

@etagwerker
Copy link
Collaborator

@etagwerker and what kind of good enough is that? Is this something you would like addressed?

I'm interested in seeing if it considerably changes the churn vs complexity graph in the projects you're working with. Once you have a branch that uses churn, I can test with my own applications and then we can discuss next steps then.

@etagwerker
Copy link
Collaborator

Also, just FYI, I don't think we need a release of churn to see this working: danmayer/churn#88 (comment)

I envision the integration will not use churn's JSON output, but it will integrate programmatically with churn

@hemalvarambhia
Copy link
Author

Also, just FYI, I don't think we need a release of churn to see this working: danmayer/churn#88 (comment)

I envision the integration will not use churn's JSON output, but it will integrate programmatically with churn

I agree. That would be a huge time saver.

@hemalvarambhia
Copy link
Author

hemalvarambhia commented Dec 4, 2024

I have a small PR (#499) , in which I add a learning test to understand how the gem works. In case anyone's not familiar with the idea of a learning test, it is to way to gain clarification on how an external library and has the bonus of documenting assumptions that can be tested when the gem is updated.

@hemalvarambhia
Copy link
Author

@etagwerker #499 contains a learning test (Test-Driven Development By Example, Kent Beck) I thought I could add as a safe microstep towards contributing to Rubycritic. It is small enough to throw away if I need to start over and hopefully easy for you to review and safe enough consider making part of the codebase.

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

No branches or pull requests

3 participants