Skip to content
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.

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

use correct instance of valueOrDefault in FormattingRule #1154

Closed
wants to merge 1 commit into from

Conversation

Mauin
Copy link
Collaborator

@Mauin Mauin commented Sep 23, 2018

Similar to #1152 I think this rule should use the ConfigAware#valueOrDefault method instead of accessing the config directly.

@arturbosch
Copy link
Member

arturbosch commented Sep 23, 2018

This change is actually wrong, the android property should be declared once for the ruleSet:

	/**
	 * Wrapped configuration of the ruleSet this rule is in.
	 */
	val config: Config

Just using valueOrDefault will return the subConfig of this rule while config.valueOrDefault returns properties from the ruleSet itself.
Jap, as you mentioned in #1153 and #1152 maybe we should rename config to ruleSetConfig?

@arturbosch arturbosch closed this Sep 23, 2018
@arturbosch arturbosch added this to the RC9.2 milestone Sep 23, 2018
@Mauin
Copy link
Collaborator Author

Mauin commented Sep 24, 2018

Ah, I see. Thanks @arturbosch! I just checked all accessors of the config property directly and this was the only usage in the rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants