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

Gradle include file for adding the Android dependency #7900

Merged
merged 20 commits into from Jul 16, 2019

Conversation

@cutiko
Copy link
Contributor

cutiko commented Jul 1, 2019

The objective of this pull request is to have one source for content about adding the Android dependency. Before this, there were at least 3 files with repeated the content, inconsistent or used deprecated instructions.

By adding an include _gradle.md, maintenance can be done in one file only and avoids repetitive work in the rest of the documentation.

An include file name _gradle.md was created with the needed content. The file merge the instructions previously contained in:

  • quickstart/native/android/_include/_auth0.md
  • quickstart/native/android/_include/_api_authz.md

Some edition was made to the content to make it fit between both and modularly in any other file.

In the note used before there was a link to the Github documentation. The link was removed because the documentation source file has the version hardcoded. If the documentation referred is not updated it would be inconsistent with the Maven or JCenter latest version.

A minor note about this is that the block of code was using xml instead of groovy as code block language.

While checking if there was any other repetitive usage of the Android installation explained, the reference in the file libraries/save-and-refresh-tokens.md was found. It was using compile instead of implementation, the first is deprecated. Using the include _gradle.md the file allows keeping the reference updated in a single place.

The included usage was taken literally from the documentation, but in the files is different, so I went with my guts and stick to the guidelines.

I had problems understanding the Wordy instructions for the editor, so my apologies if anything failed on that step, however, it was followed and the editor's corrections were applied.

Update yml files are included.

If this approach please you, I found a similar problem for the manifest, I could take a look later. Please let me know if there is anything else I can help with.


https://docs-content-staging-pr-7900.herokuapp.com/docs/libraries/auth0-android
https://docs-content-staging-pr-7900.herokuapp.com/docs/libraries/auth0-android/save-and-refresh-tokens
https://docs-content-staging-pr-7900.herokuapp.com/docs/quickstart/native/android

@cutiko cutiko requested a review from Jul 1, 2019
@lbalmaceda lbalmaceda requested review from lbalmaceda and removed request for Jul 2, 2019
@solepano solepano temporarily deployed to docs-content-staging-pr-7900 Jul 2, 2019 Inactive
Copy link
Contributor

lbalmaceda left a comment

I think is a great change. I've left you some comments!

Co-Authored-By: Luciano Balmaceda <balmacedaluciano@gmail.com>
@solepano solepano temporarily deployed to docs-content-staging-pr-7900 Jul 2, 2019 Inactive
Co-Authored-By: Luciano Balmaceda <balmacedaluciano@gmail.com>
@solepano solepano temporarily deployed to docs-content-staging-pr-7900 Jul 2, 2019 Inactive
Co-Authored-By: Luciano Balmaceda <balmacedaluciano@gmail.com>
@solepano solepano temporarily deployed to docs-content-staging-pr-7900 Jul 2, 2019 Inactive
@solepano solepano had a problem deploying to docs-content-staging-pr-7900 Jul 2, 2019 Failure
@cutiko
Copy link
Contributor Author

cutiko commented Jul 2, 2019

I agree with everything, thanks for pointing it out and for finding another source of improvement. My only doubt in this moment is:

Should we move the _gradle.md to libraries/android/_includes/... ?

@solepano solepano had a problem deploying to docs-content-staging-pr-7900 Jul 3, 2019 Failure
@cutiko
Copy link
Contributor Author

cutiko commented Jul 3, 2019

Greetings

Took the liberty of solving the merge conflict with my yml the other content should be included as well

I hope to help, let me know if anything else I can do.

@jeffreylees
Copy link
Contributor

jeffreylees commented Jul 8, 2019

@lbalmaceda does this look good to you now?

@lbalmaceda
Copy link
Contributor

lbalmaceda commented Jul 16, 2019

@cutiko Sorry I forgot about this one. It looks good now! Thanks for the changes.
@jeffreylees let me fix a file reference error and then this is ready to merge

@solepano solepano had a problem deploying to docs-content-staging-pr-7900 Jul 16, 2019 Failure
Jeff Smith
@jeffreylees
Copy link
Contributor

jeffreylees commented Jul 16, 2019

It builds now, and the pages load correctly - all good to merge @lbalmaceda ?

@jeffreylees jeffreylees merged commit cb68d6f into auth0:master Jul 16, 2019
2 checks passed
2 checks passed
license/snyk - package.json (auth0-core) No manifest changes detected
security/snyk - package.json (auth0-core) No manifest changes detected
@jeffreylees
Copy link
Contributor

jeffreylees commented Jul 16, 2019

Thanks @cutiko !!

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

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.