8000 Update README.md by gimbimloki · Pull Request #132 · OpenFeign/feign · GitHub
[go: up one dir, main page]

Skip to content

Update README.md#132

Merged
codefromthecrypt merged 4 commits intoOpenFeign:masterfrom
gimbimloki:patch-1
Jan 8, 2015
Merged

Update README.md#132
codefromthecrypt merged 4 commits intoOpenFeign:masterfrom
gimbimloki:patch-1

Conversation

@gimbimloki
Copy link
Contributor

Hi.

In JAX-RS section, adding 'JAXRSContract' is omitted.
So, I didn't understand the JAX-RS example correctly. And I got an exception when I run it.
If adding 'JAXRSContract' is added, It is helpful to beginners like me.

Thank you.

Hi.

In JAX-RS section, adding 'JAXRSContract' is omitted.
So, I didn't understand the JAX-RS example correctly. And I got an exception when I run it.
If adding 'JAXRSContract' is added, It is helpful to beginners like me.

Thank you.
@cloudbees-pull-request-builder

NetflixOSS » feign » feign-pull-requests #1 SUCCESS
This pull request looks good

@codefromthecrypt
Copy link
Contributor

Looks good, but let's try to keep the fole as short as possible. I think lines 149-152 are the most essential to add. Do you agree?

I removed the unessential lines. 

Thanks for your advice.
@gimbimloki
Copy link
Contributor Author

Yes, I do.
It looks much better to understand.

@cloudbees-pull-request-builder

NetflixOSS » feign » feign-pull-requests #2 SUCCESS
This pull request looks good

Remove more lines for unity? consistency.
Customization, Request Interceptors, are written as this form.
@cloudbees-pull-request-builder

NetflixOSS » feign » feign-pull-requests #3 SUCCESS
This pull request looks good

README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip If you append java to the triple backquote, it will format nicely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Last nit: I would still retain newlines after the call to each builder. Some mobile viewers will see an awkward wrap otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean this ? Two java code blocks and retaining newlines.
I need to study English more. T.T

interface GitHub {
  @GET @Path("/repos/{owner}/{repo}/contributors")
  List<Contributor> contributors(@PathParam("owner") String owner, @PathParam("repo") String repo);
}
GitHub github = Feign.builder()
                     .contract(new JAXRSModule.JAXRSContract())
                     .target(GitHub.class, "https://api.github.com");           

Copy link
Contributor

Choose a reason for hiding this comment

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

looks great!

 In JAX-RS example, two Java code blocks and retaining new lines after the call to each builder.
8000
@cloudbees-pull-request-builder

NetflixOSS » feign » feign-pull-requests #4 SUCCESS
This pull request looks good

@codefromthecrypt
Copy link
Contributor

Thanks!

codefromthecrypt pushed a commit that referenced this pull request Jan 8, 2015
@codefromthecrypt codefromthecrypt merged commit 246e01b into OpenFeign:master Jan 8, 2015
@gimbimloki
Copy link
Contributor Author

You're welcome ~
I was happy to contribute to the feign.
I'm using it for making some API clients in my company. And I want to contribute continuously!

@gimbimloki gimbimloki deleted the patch-1 branch January 9, 2015 02:54
@codefromthecrypt codefromthecrypt added this to the 7.1.0 milestone Jan 26, 2015
velo pushed a commit that referenced this pull request Oct 8, 2024
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.

3 participants

0