codeclimate / codeclimate-pmd

13 stars 7 forks source link

Discontinue ApexMetrics (Apex-only PMD) in favour of General PMD engine #26

Closed rsoesemann closed 4 years ago

rsoesemann commented 6 years ago

When Codacy didn't provide a PMD engine I created my own called https://docs.codeclimate.com/docs/apexmetrics. It was for Salesforce apex classes only and suffered from me being the bottleneck to maintain and upgrade it.

By accident I learned that Codeclimate now has an all-languages PMD engine.

If this engine works with all languages, I'd like to:

  1. Have Codeclimate take my engine out of their site. @brynary @dblandin
  2. Remove my own engine repo
  3. Get somewhat a safe feeling from Codeclimate that users of my engine are safe ;-)
  4. Take out the Codeclimate specific parameters and dependancies I introduced to PMD Apex rules
froucher commented 6 years ago

Hi @rsoesemann, I just tried to launch PMD with Apex, but checking the source code seems this engine will only work with ".java" files https://github.com/codeclimate/codeclimate-pmd/blob/3c17c1ca4eb56ff2dbfa9566c512eb15853acd61/src/Config.groovy#L78-L84

rsoesemann commented 6 years ago

Would you mind making it work with alle Salesforce related file types as well?

Maybe @filipesperandio (the most active contributor) can help us find out if there is a technical reason to make it Java-only?

filipesperandio commented 6 years ago

Hey @rsoesemann / @froucher ! Yeah, the initial work we did with PMD was intended to be a specific for java. It probably don't have to be, though, let me check with the team.

@katwchen @toddmazierski

rsoesemann commented 6 years ago

I (and @froucher) would be happy to help. When I create ApexMetrics it was my intention to be able to update it independent of the Java changes. But now I became the bottleneck for upgrades ;-)

Cameron637 commented 6 years ago

Can I just add that for those of us using Code Climate v2 ratings, ApexMetrics is broken because there is no longer a ratings option in the configuration file, and since Apex is not a supported language by Code Climate all of the Apex files get skipped in the analysis. So either a fix there or an upgrade here to support Apex is definitely a pressing concern.

dblandin commented 6 years ago

To clarify, when using the apexmetrics engine, apex files are analyzed and issues are emitted when found, but they're currently not included within a project's maintainability rating, which is backed by an internally curated 10-point maintainability checklist: https://codeclimate.com/blog/a-new-clearer-way-to-understand-code-quality.

We have plans to reintroduce ratings from third-party engines as an additional "pillar".

Cameron637 commented 6 years ago

That makes sense, but having it as a separate pillar is also going to make my job more difficult. I'm pulling ratings from the API for all of our org projects, only a few of which use Salesforce, to go into a monthly QA report, but now I'll have to figure out a way to combine the ratings from apex and the ratings from other files (like JavaScript) for our salesforce projects, so I wish you would reconsider. I understand I'm probably one of the very few that has to worry about that though.

dblandin commented 6 years ago

I'm pulling ratings from the API for all of our org projects, only a few of which use Salesforce, to go into a monthly QA report

That's very cool! I'll definitely relay your feedback internally. Thanks for the input!

rsoesemann commented 6 years ago

Thanks for those insights. @dblandin as I wrote you on Slack (with no response) I am planning to discontinue ApexMetrics as keeping this engine up to date is a too big hurdle for me. I also know that similar services like Codacy.com have internally developed PMD engines for all PMD-supported language.

So I just can repeat myself. I am willing to help with the Salesforce-specific part (Apex,...) of such a PMD engine but I will not maintain it. Is this something Codeclimate could do? I mean making its PMD engine open for Salesforce files?

froucher commented 6 years ago

Agree with Robert, Apex is not solely the case, pmd is "an extensible cross-language static code analyzer", not just for java. So, if codeclimate "PMD engine" works with all these language (python, fortran, go, cpp, groovy, and long etc.), this could be a great feature for all those languages.

On Thu, Feb 15, 2018 at 8:22 PM, Robert Sösemann notifications@github.com wrote:

Thanks for those insights. @dblandin https://github.com/dblandin as I wrote you on Slack (with no response) I am planning to discontinue ApexMetrics as keeping this engine up to date is a too big hurdle for me. I also know that similar services like Codacy.com have internally developed PMD engines for all PMD-supported language.

So I just can repeat myself. I am willing to help with the Salesforce-specific part (Apex,...) of such a PMD engine but I will not maintain it. Is this something Codeclimate could do? I mean making its PMD engine open for Salesforce files?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codeclimate/codeclimate-pmd/issues/26#issuecomment-366033636, or mute the thread https://github.com/notifications/unsubscribe-auth/AIjjj4Pqnacj_xKO379F0fFSJnce_Oa8ks5tVIPdgaJpZM4RzLmQ .

--

BBVA

Felipe Roucher Iglesias Global Channels Architecture - Customer Engagement Platforms (@) felipe.roucher@bbva.com - (T) +34 628 929 488 Ed. Vaguada - Monforte de Lemos S/N, Madrid Schedule a meeting here https://calendly.com/froucher

Before you print this message please consider if it is really necessary.

--

"Este mensaje está dirigido de manera exclusiva a su destinatario y puede contener información privada y confidencial. No lo reenvíe, copie o distribuya a terceros que no deban conocer su contenido. En caso de haberlo recibido por error, rogamos lo notifique al remitente y proceda a su borrado, así como al de cualquier documento que pudiera adjuntarse.

Por favor tenga en cuenta que los correos enviados vía Internet no permiten garantizar la confidencialidad de los mensajes ni su transmisión de forma íntegra.

Las opiniones expresadas en el presente correo pertenecen únicamente al remitente y no representan necesariamente la opinión del Grupo BBVA."

"This message is intended exclusively for the adressee and may contain privileged and confidential information. Please, do not disseminate, copy or distribute it to third parties who should not receive it. In case you have received it by mistake, please inform the sender and delete the message and attachments from your system.

Please keep in mind that e-mails sent by Internet do not allow to guarantee neither the confidentiality or the integrity of the messages sent."

dblandin commented 6 years ago

@rsoesemann I'm sorry for not getting back to you in Slack. I've been tied up with the Code Climate: Velocity release recently.

I'm definitely in favor of pulling Apex support into the PMD engine. @filipesperandio will likely take the lead on making that happen.

rsoesemann commented 6 years ago

Great!

@filipesperandio please count on me. I am really active in the PMD repo when it comes to Apex and Salesforce as you can see here: https://github.com/pmd/pmd/issues?utf8=%E2%9C%93&q=+is%3Aissue+author%3Arsoesemann+

I also helped Codacy.com to open their engine for Apex: https://github.com/codacy/codacy-pmdjava/issues?utf8=%E2%9C%93&q=+is%3Aissue+author%3Arsoesemann+

brandonmikeska commented 6 years ago

Is there a rough timeline for this? Since the Apex Metrics is discontinued, trying to see if this is a couple weeks out or months out from happening.

filipesperandio commented 6 years ago

@rsoesemann @brandonmikeska We will have to prioritize that among other work, but I don't see it happening too soon considering what is in the pipeline, sorry.

/cc @katwchen

filipesperandio commented 6 years ago

@rsoesemann @brandonmikeska Would one of you be able to open a PR in order to support apex? I don't think we gonna be able to tackle that soon, but we could coordinate the communication related to the deprecation of the old engine.

rsoesemann commented 6 years ago

@filipesperandio not sure if you are working for codeclimate... But in case you are what you are offering is not much. To somewhat publish that I (on my own) decided to not work on ApexMetrics anymore is in you own interest as a quality engine provider.

And no, I am not willing to rewrite your internal engines simply because I don't have the time for it. I think Codeclimate should as other similar companies (e.g. Codacy) have enough interest to provide its paying customers the full spectrum of PMD supported languages. Delegating this to the community sounds like to much for me and was one reason to change provider.

I can just repeat, as the person who created the PMD module for Salesforce I am willing to help out communicate and help to efficiently communicate with the PMD world. Not less and not more.

@dblandin: Please feel free to deactivate my still active Codeclimate accounts. I don't use them anymore and they were somewhat granted to me as a thankyou for keeping the engine upgraded)

filipesperandio commented 6 years ago

@rsoesemann Apologies, I definitely misunderstood the context. We should organize our work to accommodate this.

kevinohara80 commented 5 years ago

This is really a bummer that this isn't implemented yet. A temporary workaround would be simply...

 def i = files.iterator() 
 while(i.hasNext()) { 
   def name = i.next() 
   if(!name.endsWith(".java") && !name.endsWith(".cls")) { 
     i.remove() 
   } 
 } 

@filipesperandio If I were to submit this as a PR, could this be merged? This would open up CodeClimate to the vast world of Salesforce developers. It doesn't address the other languages that PMD supports but that could be added later either in a similar fashion, or something more elegant.

filipesperandio commented 5 years ago

@kevinohara80 Sorry we couldn't prioritize this work yet. Having a PR would definitely help.

cc @ale7714

kevinohara80 commented 5 years ago

@filipesperandio It's submitted!