jcabi / jcabi-beanstalk-maven-plugin

Maven Plugin for AWS Elastic Beanstalk
http://beanstalk.jcabi.com/
Other
11 stars 10 forks source link

WarFile.java:147-148: Implement validation of JSON inside... #16 #18

Closed nhekfqn closed 9 years ago

dmarkov commented 9 years ago

Let me find a reviewer for this pull request, thanks for submitting it

dmarkov commented 9 years ago

@krzyk please review

krzyk commented 9 years ago

@nhekfqn few comments above, and you should add a test for invalid json also

nhekfqn commented 9 years ago

@krzyk please review.

krzyk commented 9 years ago

@nhekfqn we are almost there :), see my comments above

nhekfqn commented 9 years ago

@krzyk

you could just add @Test(expected = MojoFailureException.class)

I know, but I couldn't check exception message in that case. MojoFailureException can be thrown from three places in WarFile.checkEbextensionsValidity so its message does matter.

Other defects fixed, please review.

krzyk commented 9 years ago

@nhekfqn You can use ExpectedException for checking message

nhekfqn commented 9 years ago

@krzyk please review.

krzyk commented 9 years ago

@nhekfqn a comment about todo formatting

nhekfqn commented 9 years ago

@krzyk Please review.

krzyk commented 9 years ago

@rultor merge pls

rultor commented 9 years ago

@rultor merge pls

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

rultor commented 9 years ago

@rultor merge pls

@krzyk Done! FYI, the full log is here (took me 8min)

dmarkov commented 9 years ago

@krzyk done, I added 17 mins in payment 51683416... extra minutes for review comments (c=2)... added +17 to your rating, now it is equal to +4210

dmarkov commented 9 years ago

@rultor deploy

rultor commented 9 years ago

@rultor deploy

@dmarkov OK, I'll try to deploy now. You can check the progress here

rultor commented 9 years ago

@rultor deploy

@dmarkov Done! FYI, the full log is here (took me 6min)