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.

Sign up for GitHub

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

Jump to bottom

Split DHCPv6 and DHCPv4 client into subpackages #241

Merged
merged 3 commits into from
Jan 28, 2019
Merged

Split DHCPv6 and DHCPv4 client into subpackages #241

merged 3 commits into from
Jan 28, 2019

Conversation

insomniacslk
Copy link
Owner

@insomniacslk insomniacslk commented Jan 27, 2019 •
edited
Loading

Fixes #231

@insomniacslk insomniacslk self-assigned this Jan 27, 2019
@stickler-ci
Copy link

Could not review pull request. It may be too large, or contain no reviewable changes.

@insomniacslk insomniacslk force-pushed the split_client_server_from_parser branch 4 times, most recently from e989747 to c04d263 Compare January 28, 2019 00:28
@insomniacslk
Copy link
Owner Author

this breaks the router7 build. I'll make a PR

@insomniacslk insomniacslk mentioned this pull request Jan 28, 2019
Merged
@codecov
Copy link

codecov bot commented Jan 28, 2019 •
edited
Loading

Codecov Report

Merging #241 into master will increase coverage by 4.09%.
The diff coverage is 26.92%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #241      +/-   ##
========================================
+ Coverage    70.9%    75%   +4.09%     
========================================
  Files          69     67       -2     
  Lines        3375   3112     -263     
========================================
- Hits         2393   2334      -59     
+ Misses        865    679     -186     
+ Partials      117     99      -18
Impacted Files Coverage Δ
netboot/netboot.go 0% <0%> (ø) ⬆️
dhcpv4/bsdp/client.go 0% <0%> (ø) ⬆️
dhcpv6/client6/client.go 3.53% <0%> (ø)
dhcpv6/client6/iputils.go 78% <100%> (ø)
dhcpv4/async/client.go 80.95% <100%> (ø) ⬆️
dhcpv6/server6/server.go 75.8% <100%> (ø)
dhcpv4/bsdp/bsdp.go 78.03% <100%> (ø) ⬆️
dhcpv6/async/client.go 70.73% <60%> (ø) ⬆️
dhcpv6/option_requestedoption.go 74.07% <0%> (-18.52%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64f2946...967efe1. Read the comment docs.

@insomniacslk insomniacslk changed the title [WIP] Split DHCPv6 and DHCPv4 client into subpackages Split DHCPv6 and DHCPv4 client into subpackages Jan 28, 2019
@insomniacslk insomniacslk force-pushed the split_client_server_from_parser branch 2 times, most recently from a68a254 to 9c6932b Compare January 28, 2019 01:06
@insomniacslk
Copy link
Owner Author

PR for router7 at rtr7/router7#23

@insomniacslk
Copy link
Owner Author

This is ready to review. The CI failures are due to router7, and fixed in the related PR

@insomniacslk insomniacslk force-pushed the split_client_server_from_parser branch from 9c6932b to a2b283d Compare January 28, 2019 10:35
pmazzini
pmazzini reviewed Jan 28, 2019
View reviewed changes
dhcpv6/client6/iputils.go Outdated Show resolved Hide resolved
@pmazzini
Copy link
Collaborator

Commented about iputils.go. The rest looks good to me.

@insomniacslk insomniacslk force-pushed the split_client_server_from_parser branch from 967efe1 to 9cbc29c Compare January 28, 2019 11:07
@insomniacslk
Copy link
Owner Author

@pmazzini done

pmazzini
pmazzini approved these changes Jan 28, 2019
View reviewed changes
Copy link
Collaborator

@pmazzini pmazzini left a comment

Choose a reason for hiding this comment

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

I wonder if we should move the BSDP client to a separate directory as well. Anyway, approving.

@insomniacslk
Copy link
Owner Author

I wonder if we should move the BSDP client to a separate directory as well. Anyway, approving.

Probably the right thing to do is to move bsdp and async under client6, otherwise leaving them in the main namespace

@insomniacslk insomniacslk merged commit 89df477 into master Jan 28, 2019
@insomniacslk insomniacslk deleted the split_client_server_from_parser branch January 28, 2019 11:21
@pmazzini pmazzini mentioned this pull request Jan 28, 2019
Closed
@hugelgupf hugelgupf mentioned this pull request Jan 29, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmazzini pmazzini pmazzini approved these changes

@tfg13 tfg13 Awaiting requested review from tfg13

@hugelgupf hugelgupf Awaiting requested review from hugelgupf

@get9 get9 Awaiting requested review from get9

Assignees

@insomniacslk insomniacslk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@insomniacslk @stickler-ci @pmazzini