Skip to content

Isolate Jersey dependency - #51

Merged
stoilkov merged 2 commits into
paymill:masterfrom
basoko:isolate-Jersey-dependency
Oct 2, 2014
Merged

Isolate Jersey dependency#51
stoilkov merged 2 commits into
paymill:masterfrom
basoko:isolate-Jersey-dependency

Conversation

@basoko

Copy link
Copy Markdown
Contributor

I have done a small refactor with the aim of isolate the Jersey dependency.

@nikoloff

Copy link
Copy Markdown
Contributor

I like this proposal.
I will touch the methods visibility and will build a new version with it.

thx

@basoko

Copy link
Copy Markdown
ContributorAuthor

Ok @nikoloff but if you want I can change the methods visibility that you think need a change and update the pull request.

Regards.

@nikoloff

Copy link
Copy Markdown
Contributor

Yes thx @basoko , that will be great.
In the services package the utilities classes ParameterMap and HttpClient with it's methods should be final with package visibility.

@basoko

Copy link
Copy Markdown
ContributorAuthor

Sorry @nikoloff, I'm not sure about what you mean because ParameterMap is final and HttpClient is an interface so it can not be final.
I can't change the package visibility because the HttpClient interface is needed in the PaymillContext class, so it has to have public visibility, and at the same time it depends on ParameterMap, so this last one should also be public.

@stoilkov

Copy link
Copy Markdown
Contributor

@basoko sorry about the merge delay, we will take care of both pull requests next week. Again, thanks for the contribution.

@basoko

Copy link
Copy Markdown
ContributorAuthor

Hi @nikoloff, @stoilkov is this PR alive?

@stoilkov

Copy link
Copy Markdown
Contributor

@basoko sorry about the delay. will take care of this today.

@basoko

Copy link
Copy Markdown
ContributorAuthor

@stoilkov Don't worry, there's no rush. I only wanted to know if there's something I can do to improve it.

@stoilkov
stoilkov merged commit 79ab6e9 into paymill:masterOct 2, 2014
Sign up for freeto 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

@basoko@nikoloff@stoilkov