-
Notifications
You must be signed in to change notification settings - Fork 4
Conversation
<application | ||
android:allowBackup="true" | ||
android:icon="@mipmap/ic_launcher" | ||
android:label="@string/app_name" | ||
android:roundIcon="@mipmap/ic_launcher_round" | ||
android:supportsRtl="true" | ||
android:theme="@style/AppTheme" > | ||
android:theme="@style/AppTheme" | ||
android:networkSecurityConfig="@xml/network_security_config"> |
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.
since network_security_config
is .gitignored, will adding it to the manifest here cause breakage if it's not present in the project (e.g.: someone forgot to add it when they cloned the repo, and then tried to run the application)?
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 not ignore the file (expose the URL in this repo)?
Also applies to the hosts files that I had added to gitignore for links that I did not want to be exposed to github(but maybe we should actually expose them)?
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 think you're ok. I see now that you've added the network_security_config
and there's plenty of warnings about not opening network activity to non-SSL endpoints in your comments. Go ahead and make an issue on your repo to expose an SSL endpoint for the product catalog so you can remove this config in the future.
4bd4beb
to
046f408
Compare
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.
LGTM. Check out my latest comment: https://github.com/fullstorydev/android-shoppe-demo/pull/13/files#r389789103
adding two utils for making network calls.
network_security_config file is required for making API call for non-ssl connections. the config file itself is gitignored and the in manifest marked as dev only