Skip to content

Prevent all reflection across the library. #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 10, 2017
Merged

Prevent all reflection across the library. #115

merged 1 commit into from
Feb 10, 2017

Conversation

pyr
Copy link
Contributor

@pyr pyr commented Feb 10, 2017

  • Standardize on *warn-on-reflection* to true for the dev profile
  • Fix metrics.health.core/check (previously called protected method check instead of execute).
  • Fix metrics.reporters.influxdb (previously called setGuages instead of setGauges)
@michaelklishin michaelklishin merged commit e140f02 into clj-commons:master Feb 10, 2017
@michaelklishin
Copy link
Collaborator

Thank you!
@pyr pyr deleted the feature/no-reflection branch February 10, 2017 14:08
@pyr
Copy link
Contributor Author

pyr commented Feb 10, 2017

Thanks, that was quick!
@michaelklishin
Copy link
Collaborator

I assume points 2 and 3 deserve a bug fix release?
@pyr
Copy link
Contributor Author

pyr commented Feb 10, 2017

Yup, I think it's warranted.
One think to arbitrate on is that the Guage/Gauge typo is still present in the
options map (i.e: it still has :keys [group-guages]). I left it there in case someone somewhere feeds :group-guages in the option map. Going from 2.8.0 to 2.9.0 might warrant fixing the spelling in options too.
@michaelklishin
Copy link
Collaborator

@pyr let's fix that and go straight to 2.9.0.
@pyr pyr mentioned this pull request Feb 10, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@pyr @michaelklishin