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

TkMethods now returns 405 on unsupported method #875

Merged
merged 1 commit into from
Dec 1, 2018

Conversation

victornoel
Copy link
Contributor

This is for #861.

Added a test and a fix.

@0crat
Copy link
Collaborator

0crat commented Nov 28, 2018

Job #875 is now in scope, role is REV

@0crat
Copy link
Collaborator

0crat commented Nov 28, 2018

This pull request #875 is assigned to @krzyk/z, here is why; the budget is 15 minutes, see §4; please, read §27 and when you decide to accept the changes, inform @paulodamaso/z (the architect) right in this ticket; if you decide that this PR should not be accepted ever, also inform the architect; this blog post will help you understand what is expected from a code reviewer; there will be a monetary reward for this job

@victornoel
Copy link
Contributor Author

@paulodamaso travis should be re-run because of #876, I don't have the access rights to do so.

@codecov-io
Copy link

codecov-io commented Nov 28, 2018

Codecov Report

Merging #875 into master will increase coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #875      +/-   ##
============================================
+ Coverage      75.3%   75.45%   +0.15%     
- Complexity      974      977       +3     
============================================
  Files           220      220              
  Lines          4717     4718       +1     
  Branches        368      368              
============================================
+ Hits           3552     3560       +8     
+ Misses         1010     1003       -7     
  Partials        155      155
Impacted Files Coverage Δ Complexity Δ
src/main/java/org/takes/facets/fork/TkMethods.java 80% <100%> (+5%) 2 <1> (+1) ⬆️
src/main/java/org/takes/tk/TkFailure.java 88.88% <0%> (+22.22%) 5% <0%> (+1%) ⬆️
src/main/java/org/takes/facets/fork/FkFixed.java 60% <0%> (+60%) 1% <0%> (+1%) ⬆️

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 6146d34...e6fc4cf. Read the comment docs.

@paulodamaso
Copy link
Contributor

@victornoel Done

@victornoel
Copy link
Contributor Author

@paulodamaso thanks, it worked :)

Copy link
Contributor

@krzyk krzyk left a comment

Choose a reason for hiding this comment

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

@victornoel please see my comments

@@ -62,4 +67,19 @@ public void throwsExceptionOnActinOnUnproperMethod() throws
new RqFake(RqMethod.GET)
);
}

/**
* TkMethods can return 405 status when acting on unknown method method.
Copy link
Contributor

Choose a reason for hiding this comment

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

@victornoel method method? :)

IOException {
new FtRemote(new TkMethods(new TkEmpty(), "PUT")).exec(
url -> new JdkRequest(url)
.method("POST")
Copy link
Contributor

Choose a reason for hiding this comment

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

@victornoel RqMetod.POST

@Test
public void returnsMethodIsNotAllowedForUnsupportedMethods() throws
IOException {
new FtRemote(new TkMethods(new TkEmpty(), "PUT")).exec(
Copy link
Contributor

Choose a reason for hiding this comment

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

@victornoel RqMethod.PUT


/**
* TkMethods can return 405 status when acting on unknown method method.
* @throws IOException if any I/O error occurs.
Copy link
Contributor

Choose a reason for hiding this comment

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

@victornoel descriptions in javadoc for throws/params/return should start with capital letter, and should not end with a dot.

@victornoel
Copy link
Contributor Author

@krzyk thanks, I've updated the commit and rebased on master

@krzyk
Copy link
Contributor

krzyk commented Dec 1, 2018

@victornoel thanks

@krzyk
Copy link
Contributor

krzyk commented Dec 1, 2018

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Dec 1, 2018

@rultor merge

@krzyk Thanks for your request. @yegor256 Please confirm this.

@paulodamaso
Copy link
Contributor

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Dec 1, 2018

@rultor merge

@paulodamaso OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit e6fc4cf into yegor256:master Dec 1, 2018
@rultor
Copy link
Collaborator

rultor commented Dec 1, 2018

@rultor merge

@paulodamaso Done! FYI, the full log is here (took me 19min)

@0crat
Copy link
Collaborator

0crat commented Dec 1, 2018

@ypshenychka/z please review this job completed by @krzyk/z, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

@0crat 0crat removed the scope label Dec 1, 2018
@0crat
Copy link
Collaborator

0crat commented Dec 1, 2018

The job #875 is now out of scope

@0crat
Copy link
Collaborator

0crat commented Dec 1, 2018

Payment to ARC for a closed pull request, as in §28: +10 point(s) just awarded to @paulodamaso/z

@ypshenychka
Copy link

@krzyk According to our QA Rules:

The code reviewer found at least three problems in the code.
Comments were mostly about design problems, not cosmetic issues.

Only two major issues were found during code review.
Please confirm that you'll try to find at least three major problems while future reviews.

@krzyk
Copy link
Contributor

krzyk commented Dec 2, 2018

@ypshenychka I confirm

@ypshenychka
Copy link

@krzyk Thanks

@ypshenychka
Copy link

@0crat quality acceptable

@victornoel victornoel deleted the 861-tkmethods-405 branch December 2, 2018 12:55
@0crat
Copy link
Collaborator

0crat commented Dec 2, 2018

Order was finished, quality is "acceptable": +15 point(s) just awarded to @krzyk/z

@0crat
Copy link
Collaborator

0crat commented Dec 2, 2018

Quality review completed: +8 point(s) just awarded to @ypshenychka/z

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.

7 participants