-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update doc for v.2.0.0 and use Core MapUtils #89
Conversation
Update implementation from 2+ to 2.0.0 Update sample code in doc.
Codecov Report
@@ Coverage Diff @@
## dev-v2.0.0 #89 +/- ##
==============================================
- Coverage 93.37% 93.36% -0.01%
==============================================
Files 12 12
Lines 679 678 -1
Branches 103 103
==============================================
- Hits 634 633 -1
Misses 16 16
Partials 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Update Readme
Use Core MapUtils for isNullOrEmpty
Documentation/api-reference.md
Outdated
@@ -134,6 +134,9 @@ Identity.getUrlVariables(new AdobeCallback<String>() { | |||
|
|||
Registers the Identity for Edge Network extension with the Mobile Core extension. | |||
|
|||
> **Warning** | |||
> Deprecated as of 2.0.0. Use [MobileCore.registerExtensions API](https://github.com/adobe/aepsdk-core-android/blob/main/docs/Usage/MobileCore.md#registering-extensions-and-starting-the-sdk) instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's remove this link to mobile core
Updatet documentation, make toc for Readme.md. Move advertising-identifier content to its own file.
Update getting-started.md
Remove setLogLevel in the sample code
Update Documentation
Fix typo in a link
Documentation/getting-started.md
Outdated
3. In your mobile property, select **Extensions** tab. | ||
4. On the **Catalog** tab, locate or search for the **Identity** extension, and select **Install**. | ||
5. There are no configuration settings for **Identity**. | ||
6. Follow the [publishing process](https://developer.adobe.com/client-sdks/documentation/getting-started/create-a-mobile-property) to update SDK configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include this Adobe.io link?
In the developer.com, it just has "Follow the publishing process ...." with no link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe remove it here too
Update descriptions for related projects
README.md
Outdated
@@ -3,82 +3,28 @@ | |||
|
|||
## About this project | |||
|
|||
The Adobe Experience Platform Edge Identity is a mobile extension for the [Adobe Experience Platform SDK](https://github.com/Adobe-Marketing-Cloud/acp-sdks) and requires the `MobileCore` extension. This extension enables handling of user identity data from a mobile app when using the AEP Mobile SDK and the Edge Network extension. | |||
The Adobe Experience Platform Edge Identity is a mobile extension for the [Adobe Experience Platform SDK](https://developer.adobe.com/client-sdks/documentation) and requires the `MobileCore` extension. This extension enables handling of user identity data from a mobile app when using the AEP Mobile SDK and the Edge Network extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Adobe Experience Platform Identity for Edge Network mobile extension enables identity management from your mobile app when using the Adobe Experience Platform Mobile SDK and the Edge Network extension.
README.md
Outdated
@@ -93,7 +39,11 @@ make init | |||
|
|||
| Project | Description | | |||
| ------------------------------------------------------------ | ------------------------------------------------------------ | | |||
| [AEP SDK Sample App for Android](https://github.com/adobe/aepsdk-sample-app-android) | Contains Android sample app for the AEP SDK. | | |||
| [AEP Core Extensions](https://github.com/adobe/aepsdk-core-android) | The AEPCore represents the foundation of the Adobe Experience Platform SDK. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- | Core extensions | The Mobile Core represents the foundation of the Adobe Experience Platform mobile SDK. |
- | Edge Network extension | The Edge Network extension allows you to send data to the Adobe Edge Network from a mobile application. |
- | Consent for Edge Network extension | The Consent ...
- | Assurance extension (link should be https://github.com/adobe/aepsdk-assurance-android) | The Assurance extension enables validation workflows for your SDK implementation.
- | AEP SDK sample app for Android | Contains the Android sample app for the AEP SDKs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the removal of the AEP acronym from the extension names on the project column side; could we extend that to the sample app row as well? Maybe something like:
Adobe Experience Platform sample app for Android
or
Adobe Experience Platform Android sample app
And the description to something like (i think we can use Experience Platform here instead of Adobe Experience Platform since it was already mentioned prominently in the Project column side):
Contains a fully implemented Android sample app using the Experience Platform SDKs.
@@ -0,0 +1,29 @@ | |||
## Troubleshooting Guides | |||
|
|||
### M1 issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Development on M1 Macs
Configure a new Assurance session by setting the Base URL to `testapp://main` and launch Assurance in the demo app by running the following command in your terminal: | ||
|
||
```bash | ||
$ adb shell am start -W -a android.intent.action.VIEW -d "testapp://main?adb_validation_sessionid=ADD_YOUR_SESSION_ID_HERE" com.adobe.marketing.mobile.testapp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this test app does not have the scheme configured for deeplinks. e.g.:
https://github.com/adobe/aepsdk-edge-android/blob/dev-v2.0.0/code/app/src/main/AndroidManifest.xml#L40-L43
Documentation/getting-started.md
Outdated
3. In your mobile property, select **Extensions** tab. | ||
4. On the **Catalog** tab, locate or search for the **Identity** extension, and select **Install**. | ||
5. There are no configuration settings for **Identity**. | ||
6. Follow the [publishing process](https://developer.adobe.com/client-sdks/documentation/getting-started/create-a-mobile-property) to update SDK configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe remove it here too
Documentation/api-reference.md
Outdated
|
||
> **Warning** | ||
>The Identity for Edge Network extension does not read the Mobile SDK's privacy status, and therefore setting the SDK's privacy status to opt-out will not automatically clear the identities from the Identity for Edge Network extension. See [`MobileCore.resetIdentities`](https://aep-sdks.gitbook.io/docs/foundation-extensions/mobile-core/mobile-core-api-reference#resetidentities) for more details. | ||
>The Identity for Edge Network extension does not read the Mobile SDK's privacy status, and therefore setting the SDK's privacy status to opt-out will not automatically clear the identities from the Identity for Edge Network extension. See [`MobileCore.resetIdentities`](https://github.com/adobe/aepsdk-core-android/blob/staging/Documentation/Usage/MobileCore.md) for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid full path and just provide the core repo url.
@emdobrin @kevinlind @cacheung @timkim I see URLs to faqs and core repos here, do you think we could have section in getting started and reference that section wherever we need links in our doc? Or should we move FAQs to repo docs as well?
Section with links |
---|
Privacy/GDPR |
Core API Ref |
EdgeIdentity FAQ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I feel a bit conflicted with only having a main table/area for all links and removing them from the text completely; on one hand, yes the burden of maintaining these links that have potential to break is reduced for us, but on the other hand, effectively the burden is transferred to users in the form of having to find that information themselves
I feel that it really reduces the 3rd party developer experience by making relevant and specific information much harder to find (for example, when mentioning an API, being unable to link to that specific API with the header deep link and having to dig through the entire core repo docs). I think this effect is exacerbated by the fact that new users who would most benefit from having this easily accessible information are most burdened because they have the least amount of understanding to know where to find information and how our extensions work.
For example, on this firebase tutorial page: https://firebase.google.com/docs/ios/installation-methods#cocoapods
under step 2, they have a link to supported products. Imagine if they threw us to the very top of the page with no context, it would be very confusing to understand that they meant the Available libraries
section
Similarly, imagine if Javadocs or Apple Swift/ObjC docs didnt take you to the specific API within a huge class page, but always placed you at the top? This kind of degraded developer experience isnt a deal breaker but it adds to the overall experience and perception of the product
Sorry for the rant, and this is definitely not to target you but I think we could benefit from a different approach, maybe even creating a script/github action that is able to look through a repo's static links and replace them with the new value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote for keeping
https://github.com/adobe/aepsdk-core-android/blob/main/Documentation/Usage/MobileCore.md at least for [MobileCore.resetIdentities
]. (I changed the branch to main now) If just going to the repo, user isn't going to find it right away. We can avoid the deep link though. This page isn't that big, user can scroll down and find it.
We can find a balance depends on the situation. For Privacy and GDPR, I don't think we should move it to this repo, so it is better to point it to the developer.com.
EdgeIdentity FAQ, good point, we should move it to this repo as well. I will copy that over.
I don't think we need to have a table with links.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am of the believer of having links to each and every section. I was just trying to see if we have a way to simplify it as per discussion in the team guild meeting. But yes I do believe we should have all the API reference directly without any redirects. And even if it is a task to update, developers should be able to move across documentation without any friction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Calise! Made some suggestions for text changes, broadly covering:
- Replacement of the AEP acronym with "Adobe Experience Platform" or "Experience Platform" (when proper context is available)
- Aligning getting started java and kotlin examples
- Fleshing out the getting started guide, particularly around setting up the mobile property for the test app
- Misc. styling and other changes
Please let me know what you think!
Documentation/getting-started.md
Outdated
|
||
## Add the AEP Identity extension to your app | ||
|
||
### Download and import the Identity extension | ||
|
||
> :information_source: The following instructions are for configuring an application using Adobe Experience Platform Edge mobile extensions. If an application will include both Edge Network and Adobe Solution extensions, both the Identity for Edge Network and Identity for Experience Cloud ID Service extensions are required. Find more details in the [Frequently Asked Questions](https://aep-sdks.gitbook.io/docs/foundation-extensions/identity-for-edge-network/identity-faq) page. | ||
> :information_source: The following instructions are for configuring an application using Adobe Experience Platform Edge mobile extensions. If an application will include both Edge Network and Adobe Solution extensions, both the Identity for Edge Network and Identity for Experience Cloud ID Service extensions are required. Find more details in the [Frequently Asked Questions](https://developer.adobe.com/client-sdks/documentation/identity-for-edge-network/faq) page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we style this using the special github notice block?
ex:
Note
text.......
Documentation/getting-started.md
Outdated
// register Adobe extensions | ||
MobileCore.registerExtensions( | ||
Arrays.asList(Edge.EXTENSION, Identity.EXTENSION), | ||
o -> Log.d("MobileApp", "Mobile SDK was initialized") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few things I think we could update for the Java section:
- The Java section is missing the variable defn for
ENVIRONMENT_FILE_ID
- Should we update this log statement to use the new Core Log service and to be more specific (also avoiding usage of the AEP acronym)
maybe we can update with a section like:
public class MobileApp extends Application {
// Set up the preferred Environment File ID from your mobile property configured in Data Collection UI
private final String ENVIRONMENT_FILE_ID = "";
@Override
public void onCreate() {
super.onCreate();
MobileCore.setApplication(this);
MobileCore.configureWithAppID(ENVIRONMENT_FILE_ID);
// Register Adobe Experience Platform SDK extensions
MobileCore.registerExtensions(
Arrays.asList(Edge.EXTENSION, Identity.EXTENSION),
o -> Log.debug("MobileApp", "MobileApp", "Adobe Experience Platform Mobile SDK initialized.")
);
}
}
Documentation/getting-started.md
Outdated
MobileCore.registerExtensions( | ||
listOf(Edge.EXTENSION, Identity.EXTENSION, Consent.EXTENSION) | ||
) { | ||
MobileCore.configureWithAppID(ENVIRONMENT_FILE_ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the Java and Kotlin examples dont follow the same code path or have the same logs; could we update either example to align with the other? for example, updating the kotlin example to something like:
class MobileApp : Application() {
// Set up the preferred Environment File ID from your mobile property configured in Data Collection UI
private var ENVIRONMENT_FILE_ID: String = ""
override fun onCreate() {
super.onCreate()
MobileCore.setApplication(this)
MobileCore.configureWithAppID(ENVIRONMENT_FILE_ID)
// Register Adobe Experience Platform SDK extensions
MobileCore.registerExtensions(
listOf(Edge.EXTENSION, Identity.EXTENSION, Consent.EXTENSION)
) {
Log.debug("MobileApp", "MobileApp", "Adobe Experience Platform Mobile SDK initialized.")
}
}
}
$ adb shell am start -W -a android.intent.action.VIEW -d "testapp://main?adb_validation_sessionid=ADD_YOUR_SESSION_ID_HERE" com.adobe.marketing.mobile.testapp | ||
``` | ||
|
||
Note: replace ADD_YOUR_SESSION_ID_HERE with your Assurance session identifier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add code styling to the ADD_YOUR_SESSION_ID_HERE
?
**Data Collection mobile property prerequisites** | ||
|
||
The test app needs to be configured with the following edge extensions before it can be used: | ||
- Mobile Core (installed by default) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a link to the Core extension's repo?:
Mobile Core (installed by default)
@@ -1,5 +1,31 @@ | |||
# Getting started with the test app | |||
|
|||
**Data Collection mobile property prerequisites** | |||
|
|||
The test app needs to be configured with the following edge extensions before it can be used: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we flesh out the instructions for setting up the mobile property, maybe extending the explanation from the base getting-started.md
's "Configure the Identity extension in Data Collection UI" section?
Just thinking from the perspective of someone who has no idea how to use our extensions this is kind of difficult to understand (where to navigate to, how to set up a mobile property correctly from start to finish, how to get the env file ID, why it's even needed), yet the env file ID from a properly configured mobile property is crucial for the app's proper functioning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added Refer to getting started for how to get the ENVIRONMENT_FILE_ID.
Documentation/getting-started.md
Outdated
3. In your mobile property, select **Extensions** tab. | ||
4. On the **Catalog** tab, locate or search for the **Identity** extension, and select **Install**. | ||
5. There are no configuration settings for **Identity**. | ||
6. Follow the [publishing process](https://developer.adobe.com/client-sdks/documentation/getting-started/create-a-mobile-property) to update SDK configuration. | ||
|
||
## Add the AEP Identity extension to your app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we update this (and other AEP usages) to be:
Adobe Experience Platform
Incorporated the review comment part 1
Update getting-started page format.
More update for the documentation
Update doc for the review comment
The sample failing for adding this, will look at this later. Remove scheme change in AndroidManifest.
Update core github api links
Add deeplink to the test app and update get started doc
Rearrange the documentation folder files based on the documentation discussion in the meeting.
Update sample code in doc.
Use Core MapUtils for isNullOrEmpty in few places.
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: