SalesforceFoundation / ApexDoc

The latest java source for ApexDoc, a tool to document your Salesforce Apex classes.
BSD 3-Clause "New" or "Revised" License
229 stars 128 forks source link

Response to Issues opened by @Bobnix #30

Closed Bobnix closed 9 years ago

Bobnix commented 9 years ago

Switched the sorted methods to a comparator. The tree map enforced unique method names so overloaded methods were getting overwritten

njjc commented 9 years ago

Hi Bob, this is great, thanks for all the contributions! @davidhabib, our resident ApexDoc expert, is out until Wednesday, so it will probably be then that he reviews your pull requests.

Bobnix commented 9 years ago

Thanks @njjc for keeping me in the loop. I have a strong Java background so this looks like a fun project to write some code for.

Bobnix commented 9 years ago

Well carp, I fully expected this to be two different pull requests

njjc commented 9 years ago

This should be fine, if we want one change and not the other we'll let you know. I believe you should make a separate branch on your fork if you want separate pull requests.

Bobnix commented 9 years ago

Good idea, next fix will get its own branch for sure

davidhabib commented 9 years ago

looks great. thanks Bob!

davidhabib commented 9 years ago

I just ran these changes locally, and I'm getting the following warnings about the comparator changes you did:

Description Resource Path Location Type Type safety: Unchecked invocation sort(List, new Comparator(){}) of the generic method sort(List, Comparator<? super T>) of type Collections ClassModel.java /ApexDoc/src/org/salesforce/apexdoc line 47 Java Problem Type safety: The expression of type new Comparator(){} needs unchecked conversion to conform to Comparator<? super MethodModel> ClassModel.java /ApexDoc/src/org/salesforce/apexdoc line 47 Java Problem Comparator is a raw type. References to generic type Comparator should be parameterized ClassModel.java /ApexDoc/src/org/salesforce/apexdoc line 47 Java Problem Type safety: Unchecked cast from Object to List ClassModel.java /ApexDoc/src/org/salesforce/apexdoc line 46 Java Problem

can you address these?

Bobnix commented 9 years ago

Ah, that ended up getting fixed in #34. I can take those changes and add them into this set as well if you wish.

davidhabib commented 9 years ago

No need. I'm making my way through your pull requests now, so I'll get to it. I've been out of town for 5 days, so I'm still a bit buried and might not get through everything this week. thanks, Dave

David Habib | Senior Force.com Developer | salesforcefoundation.org

On Thu, Feb 5, 2015 at 12:42 PM, Bob Roberts notifications@github.com wrote:

Ah, that ended up getting fixed in #34 https://github.com/SalesforceFoundation/ApexDoc/pull/34. I can take those changes and add them into this set as well if you wish.

— Reply to this email directly or view it on GitHub https://github.com/SalesforceFoundation/ApexDoc/pull/30#issuecomment-73123055 .

Bobnix commented 9 years ago

Take all the time you need. That is a rather benign warning as warnings come, more of a 'I hope you know what you are doing'