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

rename aad to EID and remove OSM #111

Merged
merged 16 commits into from
Jan 9, 2024
Merged

rename aad to EID and remove OSM #111

merged 16 commits into from
Jan 9, 2024

Conversation

mosabami
Copy link
Collaborator

@mosabami mosabami commented Jan 5, 2024

No description provided.

Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

I started, can you take the feedback provided here and apply it to the other places.

It's okay that you have "EID" in the file name (but please don't use it in any markdown or code comments (unless referencing the filename). EID is not an approved TLA for Microsoft Entra ID. If you really need it short, for code purposes, eid is fine, but entraId or entra can work in many cases as well. Just make sure those usages are limited to code/filenames and not sentences/comments :)

Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

Please apply the suggestions below, and in a follow up PR, please address the assets:

  • media/aks-eslz-architecture.png
  • materials/Overview of Entrprise Scale for AKS.pptx (and any other item in materials/ that references it (I only checked one) -- while you're at it, you could fix the typo in this filename too "Entrprise" -> "Enterprise" :)

Scenarios/AKS-on-prem/AKS on HCI or Windows Server.md Outdated Show resolved Hide resolved
Copy link
Member

@ckittel ckittel left a comment

Choose a reason for hiding this comment

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

I applied some edits (please check them out): 8620491 (#111)

Also, please see the comment in this review for a few more items to fix, but can be done in a followup PR. #111 (review)

@mosabami mosabami merged commit c520213 into main Jan 9, 2024
2 checks passed
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.

None yet

2 participants