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

refactoring project #195

Merged
merged 3 commits into from
Dec 17, 2023
Merged

refactoring project #195

merged 3 commits into from
Dec 17, 2023

Conversation

shamirShahzad
Copy link
Contributor

Moved many files from lib to lib/iputils to remove import cycles could not use lib/ip as in the code ip.* was clashing with variables so used ipUtils instead many other files such as bogon, cidr, utils_input, init, regex, string_slice, asn_str, u128 and errors file needed to be moved as well otherwise import cycle was being created at ipinfo/cli/lib the go install ./ipinfo is not working as there is a clash with github.com/ipinfo/mmdbctl/lib/cmd_read.go where there is use of an ip_list file which has been moved

Moved many files from lib to lb/iputils to remove import cycles
@UmanShahzad
Copy link
Contributor

Can we rename to iputil without the caps? Would match stuff like ioutil that is an official Go package, for example.

@shamirShahzad
Copy link
Contributor Author

sure i will add that and create a pull request on mmdbctl which should be just one file so that i can install the ipinfo with go install ./ipinfo to test whether everthing is working can you confirm that one @UmanShahzad

Copy link
Contributor

@UmanShahzad UmanShahzad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fairly clean, just fix the naming to be iputil (lowercased and without suffix s), and review comments above, and Ill do one test spin

lib/ipUtils/regex.go Outdated Show resolved Hide resolved
lib/ipUtils/cidr.go Outdated Show resolved Hide resolved
lib/ipUtils/cidr.go Outdated Show resolved Hide resolved
@shamirShahzad
Copy link
Contributor Author

@UmanShahzad I did all the things kindly check if I missed anything

@UmanShahzad UmanShahzad merged commit 899bac9 into ipinfo:master Dec 17, 2023
@shamirShahzad shamirShahzad deleted the refactor branch December 18, 2023 04:35
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.

2 participants