search

RusticiSoftware / TinCanJava

Tin Can Java Library
http://rusticisoftware.github.io/TinCanJava/
Apache License 2.0
44 stars 46 forks source link

Add iterable interface to LanguageMap #24

Closed dacdave closed 10 years ago

dacdave commented 10 years ago

Add iterator class in LanguageMap and a few convenience methods.

brianjmiller commented 10 years ago

We can't just remove the lines from the pom.xml as they are needed elsewhere for generating the API documentation. It likely will work to move them elsewhere in the file, but need to do that before we just drop them outright. See #13.

brianjmiller commented 10 years ago

Do we need the MANIFEST.MF? It seems like a default one (matching the included one) will be created at package time.

brianjmiller commented 10 years ago

The style of the rest of the code needs to be strictly followed, I didn't look super closely because the tabs vs. spaces make it very difficult to review because the indenting is thrown off on github. Can you switch to using 4 spaces instead of tabs and then I'll have another look? I'm happy to have progress on the map stuff. Also note that we'll be pushing through a big PR with a backwards compatibility breakage on the RemoteLRS, basically we completely rewrote it. See that PR here: https://github.com/RusticiSoftware/TinCanJava/pull/23

I'd like to get this one in ahead of it so you can work off of it without needing to fix all your code, but be aware that that one will be landing soon. In general it provides a lot more functionality, and the interface is greatly improved, but there was no way to maintain the old API without departing significantly from the other libs which is a detriment to maintainability.

dacdave commented 10 years ago

Brian,

I only meant to include the changes to the language map code. All of the other stuff slipped through because of my lack of experience with git and github.

I will rework and resubmit this pull request without tabs, as you have requested. Please reject this entire pull request.

Thank you for your review and comments.

Dave Smith Brindle Waye 866-522-9839 ext. 823 The Design-a-Course Team The Easiest Way to Train Anyone... Anywhere! On 2014 6 10 13:37, "Brian J. Miller" notifications@github.com wrote:

Do we need the MANIFEST.MF? It seems like a default one (matching the included one) will be created at package time.

— Reply to this email directly or view it on GitHub https://github.com/RusticiSoftware/TinCanJava/pull/24#issuecomment-45646458 .

brianjmiller commented 10 years ago

Thanks and no worries, look forward to revised one. If you are using the command line, git add -ip is a handy way to quickly review what you are adding to the index.