jamessimone / apex-rollup

Fast, configurable, elastically scaling custom rollup solution. Apex Invocable action, one-liner Apex trigger/CMDT-driven logic, and scheduled Apex-ready.
MIT License
219 stars 30 forks source link

Feature/promote rollup logger visibilities #585

Closed jasonsiders closed 5 months ago

jasonsiders commented 5 months ago

Hi James 👋🏼 I'm a big fan of your work in general, and this package specifically.

My organization uses the namespaced version of the rollup package, and we recently discovered that we cannot currently utilize the logger plugin functionality detailed in the readme, because the types aren't visible. I assume this isn't a problem for most users w/the non-namespaced package.

This PR fixes this, by updating the visibilities of the RollupLogger's virtual methods, and its inner ILogger interface to be global. I didn't update the other public methods in this class, since they're not strictly necessary for what I'd like to accomplish -- but I'd be open to changing that if you think that would be helpful for others.

Here are the errors I received when trying to create my own plugin, using the latest version of the namespaced package (1.1.17.0):

When extending please.RollupLogger:

| Type  Name             Problem                                                                                                              Line:Column 
| ───── ──────────────── ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────── ─────────── 
| Error RollupLogAdapter @Override specified for non-overriding method: void RollupLogAdapter.log(String, Object, System.LoggingLevel) (7:26) 7:26        
| Error RollupLogAdapter @Override specified for non-overriding method: void RollupLogAdapter.save() (16:26)                                  16:26      

When implementing please.RollupLogger.ILogger:

Type  Name             Problem                                                  Line:Column 
| ───── ──────────────── ──────────────────────────────────────────────────────── ─────────── 
| Error RollupLogAdapter Type is not visible: please.RollupLogger.ILogger (21:14) 21:14      

Thanks for your consideration, and please let me know if you have any questions.

jamessimone commented 5 months ago

@jasonsiders thanks for the kind words, and for submitting - can you repoint to v1.6.24 as the branch? Thank you!

edit - ended up being able to make that change myself. Thanks again for contributing, I'll be rolling this out shortly!

jasonsiders commented 5 months ago

Thanks you @jamessimone!