Fix: Major restructuring for fixing the linting workflow errors and module style code writing. #266
Conversation
…r that appears in tests goes away. TODO: Maybe implement it with interfaces.
… are no clashes with the definition of Nodes - can be simplified using an interface
…redeclaration issues
…t already existed in the math directory which I updated in the previous commit
… for a lot of functions
| @@ -3,10 +3,10 @@ | |||
| package rot13 | |||
|
|
|||
| import ( | |||
| "TheAlgorithms/Go/ciphers/caesar" | |||
| "github.com/tjgurwara99/Go/ciphers/caesar" | |||
There was a problem hiding this comment.
| "github.com/tjgurwara99/Go/ciphers/caesar" | |
| "github.com/TheAlgorithm/Go/ciphers/caesar" |
|
|
||
| n := p * q | ||
| // func main() { |
There was a problem hiding this comment.
Do let me know if you think that the main function that have been commented out should be removed completely.
I kept them here because I think they are useful for making new test cases, but a lot of them already have test files so I wasn't sure I should completely remove these blocks.
There was a problem hiding this comment.
Yes it should be removed completely.
| // "fmt" | ||
| // ) | ||
|
|
||
| // func main() { |
There was a problem hiding this comment.
This file is now redundant except for maybe using this to make test cases the way it's developer meant for it to be used.
| @@ -1,11 +1,13 @@ | |||
| package main | |||
| package rsacipher | |||
There was a problem hiding this comment.
This package fails the automated tests for the encryption and decryption. I thought it was failing because the implementation of modular exponentiation was incorrect but the test for modular exponentiation passes without issues so I'm actually not sure what the problem is. My current theory is that the inverse exponentiation function is incorrect but I did't try to test it.
| @@ -1,10 +1,10 @@ | |||
| package main | |||
| package bigrsacipher | |||
There was a problem hiding this comment.
The test for this file passes but because of the automated command in the golangtest.yml file, this folder is also skipped because it shares the same prefix as rsa folder.
| @@ -0,0 +1,104 @@ | |||
| package singlylinkedlist | |||
|
|
|||
| // TODO not needed because there is already an implementation of SingleLinkedList | |||
There was a problem hiding this comment.
comment is self-explanatory
| @@ -1,3 +1,5 @@ | |||
| module TheAlgorithms/Go | |||
| module github.com/tjgurwara99/Go | |||
There was a problem hiding this comment.
| module github.com/tjgurwara99/Go | |
| module github.com/TheAlgorithms/Go |
There was a problem hiding this comment.
The benefit of this can be seen in the rot13.go file in the cipher directory
| }//end of monteCarlo | ||
| fmt.Println(monteCarlo(2<<18, cpus)) | ||
| } | ||
| // func main() { |
There was a problem hiding this comment.
As you can see this is fully a main module and would create a lot of conflicts. If I get some time this week, will rewrite it similar to other files.
|
LGTM |
|
Congratulations! This is a huge step forward. My only regret is that we have a ton of code that does not pass
It looks like this PR comments out those lines instead of running them with |
|
closes: #198 |
Apologies for this big PR but I thought I should try and fix the automated issues that this repository faces.
I have refactored quite a lot of code in order for this repository to work the way I think the Go implementation is meant to be. For example, you can work with any function in this repository as long as you do
go get GitHub.com/tjgurwara99/Go. The go mod file should be changed so that people can usego getcommand with TheAlgorithm handle. This particular feature helps avoid the presence of repeated code - however, there are still quite a lot of functions like max, min etc being redeclared locally in each package. Maintainer's should consider adding a proper contribution guidelines file which may explain how people should go about making the code more modular.As I said before I have restructured quite a lot of things in this repository and there are a lot of test files missing. Once I get time, I will try and write some more tests for the repository. But so far with all the tests that exists, there are no issues except for the
strings/sbomsubmodule test andcipher/rsasubmodule - I believe that the implementations are incorrect, but I didn't have time to check properly. All the automated linting tests with golangci-lint are running without any issues (thus we don't need any || true) in the workflow files.Also note that there are some files that were completely written as a main module which makes
go testgo crazy so I have decided to comment the code out for now and as soon as I get some time, I will try and convert them into functions and write tests so that the whole repository management can be completely automated. There are quick fixes which can sometime cause issues long term but I didn't want to cause any issues for the repository so I refrained from doing that.Hope it helps the maintainers 😄
I also wanted to ask the maintainers if the commented out code can be removed (the files where there are some tests and the main function that is commented out is not required)?