feat: add findLongestRecurringCycle algorithm#1733
feat: add findLongestRecurringCycle algorithm#1733abda-gaye wants to merge 3 commits intoTheAlgorithms:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1733 +/- ##
==========================================
+ Coverage 84.76% 84.86% +0.10%
==========================================
Files 378 380 +2
Lines 19742 19874 +132
Branches 2955 2978 +23
==========================================
+ Hits 16735 16867 +132
Misses 3007 3007 ☔ View full report in Codecov by Sentry. |
|
add findLongestRecurringCycle algorithm with tests |
appgurueu
left a comment
There was a problem hiding this comment.
The logic checks out. However:
- This also adds a solution for problem 27, which should be in a different PR.
- There are many redundant comments. There should probably be half as many comments, if not much fewer. To give just one example of a redundant comment, consider
let maxCycleLength = 0 // Store the maximum cycle length found. - The tests are also pretty repetitive. They should just use
it.each.
|
my function answers problem026
Le dim. 20 oct. 2024 à 18:19, Lamine Gaye ***@***.***> a écrit :
… it looks like you don't want my participation thank you ciao
Le dim. 20 oct. 2024 à 18:17, Lars Müller ***@***.***> a
écrit :
> ***@***.**** requested changes on this pull request.
>
> The logic checks out. However:
>
> - This also adds a solution for problem 27, which should be in a
> different PR.
> - There are *many* redundant comments. There should probably be half
> as many comments, if not much fewer. To give just one example of a
> redundant comment, consider let maxCycleLength = 0 // Store the
> maximum cycle length found.
> - The tests are also pretty repetitive. They should just use it.each.
>
> —
> Reply to this email directly, view it on GitHub
> <#1733 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AZGAU5LJLWFRNLPBAJLS62LZ4PXTVAVCNFSM6AAAAABQBXRF6GVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBQGY2DSNBZGQ>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
I'm not sure how you arrived at this conclusion. I reviewed your participation because I wouldn't want it to merge as-is yet. |
|
so give me the changes I need to make, I made 3 pull requests and since
then I haven't seen a response, I even thought that the repository is no
longer maintained
Le dim. 20 oct. 2024 à 18:26, Lars Müller ***@***.***> a
écrit :
… it looks like you don't want my participation thank you ciao
I'm not sure how you arrived at this conclusion. I reviewed your
participation because I wouldn't want it to merge as-is yet.
—
Reply to this email directly, view it on GitHub
<#1733 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AZGAU5JH32VLUFJWX75COFTZ4PYU3AVCNFSM6AAAAABQBXRF6GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRVGE3DSMBTG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I already explained what you need to change. Is there anything you don't understand about my review comments?
As far as I'm concerned, I only see you having made a single pull request in a repository I maintain: This one. |
Describe your change:
Checklist:
Problem026.jsandProblem026.test.jsare allowed.Fixes: #{$ISSUE_NO}Algorithm Description
I have implemented a new algorithm in the
Problem026.jsfile, along with a corresponding test fileProblem026.test.jsto ensure its functionality. This algorithm addresses the problem described in the following link: Project Euler Problem 26.