Skip to content

Citra update instructions #493

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

Merged
merged 4 commits into from
Jul 10, 2024
Merged

Conversation

shubham7109
Copy link
Collaborator

Description

PR to update API key instructions to point to the new developer Create an API Key page.

Links and Data

Issue: kotlin/issues/3981

@shubham7109 shubham7109 self-assigned this Jul 3, 2024
Copy link

@maryharvey maryharvey left a comment

Choose a reason for hiding this comment

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

@shubham7109 This looks good and the links work.

@maryharvey
Copy link

@shubham7109 Do you have somewhere that tells the developers how to create an API Key and where to use it?

@shubham7109
Copy link
Collaborator Author

Thanks @maryharvey!

We instruct users to create their own local.properties to locally manage their API Key, we don't provide any doc in the project on how to create the key:

@maryharvey
Copy link

maryharvey commented Jul 8, 2024

Thanks @maryharvey!

We instruct users to create their own local.properties to locally manage their API Key, we don't provide any doc in the project on how to create the key:

OK understood. The https://developers.arcgis.com/documentation/security-and-authentication/api-key-authentication/ (that you have) takes the dev to the https://developers.arcgis.com/documentation/security-and-authentication/api-key-authentication/tutorials/create-an-api-key/ tutorial. FYI - If you ever wanted to be explicit anywhere in your advice, you could put a link directly to that Tutorial to Create an API Key.
Thanks

@shubham7109
Copy link
Collaborator Author

Thanks @maryharvey Added the relevant doc here:

# Place default values for secrets needed at runtime here.
# Place real secrets in local.properties this file is committed to git.
# suppress inspection "UnusedProperty" for the whole file
# Go to https://links.esri.com/create-an-api-key to obtain a new API key access token.
API_KEY=XX

Copy link

@maryharvey maryharvey left a comment

Choose a reason for hiding this comment

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

LGTM

@eri9000 eri9000 self-requested a review July 8, 2024 22:08
Copy link
Collaborator

@eri9000 eri9000 left a comment

Choose a reason for hiding this comment

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

One suggestion for the doc of the secrets file.

Comment on lines 19 to 22
# Place default values for secrets needed at runtime here.
# Place real secrets in local.properties this file is committed to git.
# suppress inspection "UnusedProperty" for the whole file
# Go to https://links.esri.com/create-an-api-key to obtain a new API key access token.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is worth to add more explanation here:

Suggested change
# Place default values for secrets needed at runtime here.
# Place real secrets in local.properties this file is committed to git.
# suppress inspection "UnusedProperty" for the whole file
# Go to https://links.esri.com/create-an-api-key to obtain a new API key access token.
# This properties file contains default values for runtime secrets.
# Actual secrets should be placed in `local.properties` to avoid committing sensitive information.
# This file is tracked by git; `local.properties` is not.
#
# For a new API key, visit: https://links.esri.com/create-an-api-key
#

@shubham7109 shubham7109 requested a review from eri9000 July 9, 2024 14:41
@shubham7109 shubham7109 merged commit a692aa3 into v.next Jul 10, 2024
@shubham7109 shubham7109 deleted the shubham/citra-update-instructions branch July 10, 2024 16:33
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.

3 participants