Skip to content

Conversation

defaude
Copy link
Contributor

@defaude defaude commented Oct 2, 2021

Describe your change:

Refactored the test for the Backtracking/Sudoku algorithm out into a proper jest test file.

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new JavaScript files are placed inside an existing directory.
  • All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
    Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

This replaces the "main" test in the implementation file.
@pomkarnath98
Copy link
Member

If the algorithm is working fine, then no need to modify it. Someone from the team has already approved and merged by reviewing it.
Moreover, don't create test cases for other's files. Try to create PR with a new Algorithm problem.

@pomkarnath98 pomkarnath98 added the on hold Being discussed by the maintainers label Oct 3, 2021
@defaude
Copy link
Contributor Author

defaude commented Oct 3, 2021

Hi @pomkarnath98! I did not change the original implementation. I only removed the test / driver code from the implementation file and replaced it with a proper (more in-depth) test case.

Why would you dis-encourage people to contribute to already-existing implementations? :( The contribution doc explicitly encourages that.

@raklaptudirm
Copy link
Member

@pomkarnath98 The repositories of TheAlgorithms are for educational purposes. Here are the reasons why tests are useful even for perfectly coded algorithms:

  1. Tests show the learner how to use a particular algorithm.
  2. It shows where the algorithm can and should fail.
  3. It makes it easier for the maintainers to make sure changes to an algorithm do not break it.

@raklaptudirm
Copy link
Member

@defaude Please format your code with standard.js.

@raklaptudirm raklaptudirm added code style issue Failing style checks Reviewed and removed on hold Being discussed by the maintainers labels Oct 3, 2021
@pomkarnath98
Copy link
Member

@defaude I'm really thankful that people like you are getting time to contribute to open sources.
Please, don't take my words in the wrong way. But if you can find any improvement anywhere, you could've raised an issue and then resolved the issue by making this PR.
Your description itself was Refactored.. so that I got confused there.
Anyway, format your code. We'll review and merge :)

Your points are welcome @raklaptudirm.

@pomkarnath98 pomkarnath98 merged commit 94ff1e3 into TheAlgorithms:master Oct 3, 2021
@pomkarnath98 pomkarnath98 removed the code style issue Failing style checks label Oct 3, 2021
@defaude defaude deleted the test/Backtracking/Sudoku branch October 3, 2021 16:05
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