search

jcabi / jcabi-manifests

Java library for convenient reading of MANIFEST.MF files available in classpath
https://manifests.jcabi.com
Other
60 stars 22 forks source link

Add method to retreive values from several manifests #33

Closed nesterov-n closed 8 years ago

nesterov-n commented 8 years ago

This reffers #31 Add static method returning collections of values with specified key defined in several manifests

dmarkov commented 8 years ago

@nesterov-n Many thanks for the PR, let me find a reviewer for it

dmarkov commented 8 years ago

@mkordas please review, thanks

mkordas commented 8 years ago

@nesterov-n I'm on it

mkordas commented 8 years ago

@nesterov-n see my review above

mkordas commented 8 years ago

@nesterov-n ping

mkordas commented 8 years ago

@nesterov-n do you need any help?

nesterov-n commented 8 years ago

@mkordas Sorry for delay. I fixed your hints, please check

mkordas commented 8 years ago

@nesterov-n I'll check it now

mkordas commented 8 years ago

@nesterov-n see my comments above

nesterov-n commented 8 years ago

@mkordas Thanks for hints. Fixed

mkordas commented 8 years ago

@nesterov-n I'm checking

mkordas commented 8 years ago

@nesterov-n just comments about matchers, all else is good

nesterov-n commented 8 years ago

@mkordas Done.

P.S Is there some instrument to check code style: line breaks, indentation, etc? Seems that qulice doesn't cover all cases. Maybe someone has xml with code style for IDEA or some other tool? Don't want to spend so many your time for further pull requests.

mkordas commented 8 years ago

@nesterov-n there are still two comments open, there is no need to assert size when you assert using contains

mkordas commented 8 years ago

@nesterov-n this project just uses old Qulice, but even in latest version not all features are present already. IDEA settings are here: http://img.teamed.io/settings.jar

nesterov-n commented 8 years ago

@mkordas Sorry, missed some of your comments. Removed redundant size matchers from tests. Many thanks for IDEA settings.

mkordas commented 8 years ago

@nesterov-n thanks, looks good!

mkordas commented 8 years ago

@rultor merge

rultor commented 8 years ago

@rultor merge

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

yegor256 commented 8 years ago

@nesterov-n I don't really like this... instead of doing this, we should change the way we store data. we need to store them in Map<String, List<String>>

nesterov-n commented 8 years ago

@yegor256 Can you close this PR? I will make new one

mkordas commented 8 years ago

@nesterov-n generally the rule is to have just one PR (http://at.teamed.io/qa.html), so I'm not sure if @yegor256 is OK to continue in another one

nesterov-n commented 8 years ago

@mkordas No problem I will update this PR when issue #31 is assigned to me again

mkordas commented 8 years ago

@nesterov-n I think we need to close this one. There's limit of 2 tasks in this project, but one slot is taken by this PR...

nesterov-n commented 8 years ago

@mkordas no problem

dmarkov commented 8 years ago

@mkordas Thanks so much! Your account was topped up for 55 mins (transaction ID is 5715fd2e00d77d2ec2000502, the task took 260 hours and 18 mins); you're getting extra minutes here (c=40); +55 to your rating, your total score is +8302