Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Update Javadoc when setting custom attributes for users. #53

Merged
merged 4 commits into from
May 28, 2016

Conversation

drichelson
Copy link
Contributor

Update Javadoc when setting custom attributes for users.

@jkodumal
Copy link
Contributor

I think we should add in that runtime check / logging we discussed as part of this PR.

…ibute. Fix formatting. Fix null check in user builder.
@@ -0,0 +1,14 @@
package com.launchdarkly.client;

public enum UserAttribute {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should have package level access.

@jkodumal
Copy link
Contributor

👍 after changing the visibility of the new enum.

* @param k the key for the custom attribute
* Add a {@link java.lang.String}-valued custom attribute.
*
* @param k the key for the custom attribute. When set to one of the built-in user attribute keys, this custom attribute will be ignored.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps consider referencing the documentation directly on which keys are considered "built-in" (http://docs.launchdarkly.com/docs/targeting-users#targeting-based-on-user-attributes)?

Given that UserAttribute is package private it would be good to have an explicit reference to all attributes are considered "built in".

@jkodumal
Copy link
Contributor

Yes-- let's reference the documentation here.

@drichelson drichelson merged commit 3de3510 into master May 28, 2016
@drichelson drichelson deleted the dr/updateCustomDocs branch May 28, 2016 20:58
eli-darkly added a commit that referenced this pull request Mar 1, 2018
…-attrs

avoid throw/catch when evaluating user attributes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants