Conversation
jmprieur
left a comment
There was a problem hiding this comment.
Lgtm
I proposed a suggestion
jmprieur
left a comment
There was a problem hiding this comment.
Lgtm
I proposed a suggestion
| # "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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
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?