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

Remove unused code #104

Closed
normanmaurer opened this issue Feb 3, 2016 · 8 comments
Closed

Remove unused code #104

normanmaurer opened this issue Feb 3, 2016 · 8 comments
Assignees
Milestone

Comments

@normanmaurer
Copy link
Member

I would like to propose removing all code that we not need for netty (like Socket, Multicast and many more). We are only interested in the parts that are needed for our OpenSslEngine anyway. This will help us to focus on things we care about and not ship things that we are not using.

@nmittler @Scottmitch @trustin WDYT ?

@normanmaurer normanmaurer self-assigned this Feb 3, 2016
@trustin
Copy link
Member

trustin commented Feb 3, 2016

+1

1 similar comment
@nmittler
Copy link
Member

nmittler commented Feb 3, 2016

+1

@Scottmitch
Copy link
Member

+1. I think tcnative "upstream" has rebased off of our code anyways, which may make taking updates a bit more complicated but I'm not sure if we plan on taking updates.

@normanmaurer
Copy link
Member Author

Ok will take care of this

normanmaurer added a commit that referenced this issue Feb 25, 2016
Motivation:

We only use a sub-set of the original tcnative code that we forked. We should remove everything we not care about.

Modification:

Remove all stuff that we not need and want to maintain.

Result:

Less code to maintain.
normanmaurer added a commit that referenced this issue Mar 5, 2016
Motivation:

We only use a sub-set of the original tcnative code that we forked. We should remove everything we not care about.

Modification:

Remove all stuff that we not need and want to maintain.

Result:

Less code to maintain.
@normanmaurer normanmaurer modified the milestone: 1.1.33.Fork17 May 19, 2016
@nmittler
Copy link
Member

nmittler commented Jun 2, 2016

@normanmaurer did these commits make it into the 1.1.33 branch? I don't see them in the history

@normanmaurer
Copy link
Member Author

@nmittler nope... I need to rebase it figure out what I broke ;)

@nmittler
Copy link
Member

nmittler commented Jun 2, 2016

ah ok sgtm

@davidben
Copy link
Contributor

davidben commented Jun 2, 2016

I started poking at this myself. (I'd rather you all not hard-code the values of SSL_OP_* constants and figured I ought to get dead code out of the way before tackling that.) GitHub's diff viewer is really slow on that patch and I'm not sure how to download it to be sure, but I think the issue is you removed tcn_ThrowAPRException which is called a lot in ssl.c. That, in turn, requires that Error.java stay there.

normanmaurer added a commit that referenced this issue Jun 12, 2016
Motivation:

We only use a sub-set of the original tcnative code that we forked. We should remove everything we not care about.

Modification:

Remove all stuff that we not need and want to maintain.

Result:

Less code to maintain.
normanmaurer added a commit that referenced this issue Jun 16, 2016
Motivation:

We only use a sub-set of the original tcnative code that we forked. We should remove everything we not care about.

Modification:

Remove all stuff that we not need and want to maintain.

Result:

Less code to maintain.
normanmaurer added a commit that referenced this issue Jun 16, 2016
Motivation:

We only use a sub-set of the original tcnative code that we forked. We should remove everything we not care about.

Modification:

Remove all stuff that we not need and want to maintain.

Result:

Less code to maintain.
@normanmaurer normanmaurer modified the milestones: next, 1.1.33.Fork18 Jun 16, 2016
@normanmaurer normanmaurer modified the milestones: 2.0.0.Final, 2.0.0.Beta1 Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants