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

Mention how to use a CIAM tenant #105

Merged
merged 1 commit into from
May 14, 2023
Merged

Mention how to use a CIAM tenant #105

merged 1 commit into from
May 14, 2023

Conversation

rayluo
Copy link
Contributor

@rayluo rayluo commented May 10, 2023

This change is expected to be picked up by this doc.

@jmprieur , you may want to proofread the wording. (I do not have Katherine's github account. I'll ping her offline.)

@pamelafox , sorry for not including this in my previous PR, and now increasing your docs workload. We were waiting for a green light, and now (since yesterday 5/9) other relevant CIAM samples are also moving to public. Also we may want to have the relevant docs updated, before a potential content freeze. BTW, do I need to create yet another tag 0.7.0 after merging this PR here?

Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

Lgtm
I proposed a suggestion

.env.sample Outdated Show resolved Hide resolved
Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

Lgtm
I proposed a suggestion

Copy link
Contributor

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
Awesome, @rayluo !

@rayluo rayluo merged commit c1f1040 into main May 14, 2023
@rayluo rayluo deleted the mention-ciam branch May 14, 2023 06:28
# "https://login.microsoftonline.com/TENANT_GUID"
# or "https://login.microsoftonline.com/subdomain.onmicrosoft.com".
#
# If you are using a CIAM tenant, configure it as "https://subdomain.ciamlogin.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern I have with having so much information inside the code sample is internationalization- I assume that this will still show up as English when embedded in non-en translated sites, unless they have some approach to translate it. I don't know if there's a single place to put this info on learn.com, however, so maybe this is the best approach of the options.

Copy link
Contributor Author

@rayluo rayluo May 14, 2023

Choose a reason for hiding this comment

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

I understand where you are coming from, and it is true all the comments in this env.sample could show up as-is when embedded in non-en translated sites (more on this in a minute). I think that is fine in a sense that the programming source code is historically English-dominant, and this env.sample configuration file is technically still part of the source code.

I would still suggest to keep these comments here as the single source of truth which will more likely stay in-sync with the underlying sample logic, simply because they are in the same repo.

Note that the English comments inside this configuration file does not prevent the content team from referencing line 16 #AUTHORITY=<authority url> ONLY, while maintaining multi-language instructions on learn.microsoft.com, should they choose to do so.

So, now you can decide whether to go with the aforementioned "referencing line 16 only" approach, or to render the entire env.sample as-is (so that you do not have to update the quickstart doc as often).

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