8000 Send Accept header on client API calls by csetera · Pull Request #120 · katharsis-project/katharsis-framework · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Send Accept header on client API calls #120

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
Merged
merged 2 commits into from
Oct 16, 2016
Merged

Send Accept header on client API calls #120

merged 2 commits into from
Oct 16, 2016

Conversation

csetera
Copy link
Contributor
@csetera csetera commented Sep 11, 2016

As discussed in #117 (comment)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 89.92% when pulling 6499076 on Baseventure:develop into efab540 on katharsis-project:develop.

@masterspambot
Copy link
Member
masterspambot commented Sep 12, 2016 •
edited
Loading

@csetera Could you update unit tests to ensure that accept header is returned?

@csetera
Copy link
Contributor Author
csetera commented Sep 13, 2016

I will try to find some time.

@csetera
Copy link
Contributor Author
csetera commented Sep 17, 2016

Looking at this, I'm not entirely sure how to test accept headers? It looks like all of this boils down to standard JAX-RS calls via the KatharsisFeature? I'm happy to add some tests if someone points me in the right direction.

@chb0github
Copy link
Contributor
8000

The way to test this is to fire off a basic http server as part of the
test. Look at the jersey example. Then in the server, assert that the
accept header is correct

On Sat, Sep 17, 2016, 4:40 PM Craig Setera notifications@github.com wrote:

Looking at this, I'm not entirely sure how to test accept headers? It
looks like all of this boils down to standard JAX-RS calls via the
KatharsisFeature? I'm happy to add some tests if someone points me in the
right direction.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#120 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABaI0JBuHOD5u4y4pvaJeUQfNgbJCz51ks5qrHp9gaJpZM4J6Etf
.

@csetera
Copy link
Contributor Author
csetera commented Sep 18, 2016

I just pushed updates to support validating request headers. I know they are in our forked repo, but not sure if they automatically show up in this pull request? (Seems like, but if not please let me know).

@remmeier
Copy link
Contributor
remmeier commented Oct 5, 2016

can we updated the pr? then I merge it in.

@csetera
Copy link
Contributor Author
csetera commented Oct 5, 2016

I apologize for being a newbie on this. Can you tell me what is necessary to update the PR? I was under the impression it was "automatic", but if you tell me what I need to do (or point me to appropriate documentation), I'm happy to do what is necessary.

@chb0github
Copy link
Contributor

All you need to do is push to the fork/branch you used for the pr and its
automatic

On Wed, Oct 5, 2016, 12:18 PM Craig Setera notifications@github.com wrote:

I apologize for being a newbie on this. Can you tell me what is necessary
to update the PR? I was under the impression it was "automatic", but if you
tell me what I need to do (or point me to appropriate documentation), I'm
happy to do what is necessary.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#120 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABaI0Avd7U9FHbJ6gXM2U5pj9ugFGyKlks5qw_gLgaJpZM4J6Etf
.

@csetera
Copy link
Contributor Author
csetera commented Oct 6, 2016

Somehow, I must have done something wrong? I made this change and pushed it to my fork 18 days ago (see Baseventure@f37783a) . Is there a step I'm missing here?

@chb0github
Copy link
Contributor

Look for me on gitter later

On Thu, Oct 6, 2016, 4:16 AM Craig Setera notifications@github.com wrote:

Somehow, I must have done something wrong? I made this change and pushed
it to my fork 18 days ago (see Baseventure/katharsis-framework@f37783a
Baseventure@f37783a)
. Is there a step I'm missing here?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#120 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABaI0O6Y8FrE627uI3-zjkLcF2nN8v7Jks5qxNhygaJpZM4J6Etf
.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.563% when pulling 4e973c0 on Baseventure:develop into 72ee527 on katharsis-project:develop.

@csetera
Copy link
Contributor Author
csetera commented Oct 9, 2016

I think I finally got this straightened out. Please let me know if there is anything else I need to do.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 90.372% when pulling 4e973c0 on Baseventure:develop into 72ee527 on katharsis-project:develop.

@remmeier
Copy link
Contributor
remmeier 78F1 commented Oct 16, 2016

builds looks ok, sonar setup issues in the build, applying the PR anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@csetera @coveralls @masterspambot @chb0github @remmeier
0