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

Add section about variance and positions #6105

Merged
merged 12 commits into from
Oct 16, 2024
Merged

Conversation

eernstg
Copy link
Member

@eernstg eernstg commented Sep 24, 2024

@dart-github-bot
Copy link
Collaborator

dart-github-bot commented Sep 24, 2024

Visit the preview URL for this PR (updated for commit 3dfa993):

https://dart-dev--pr6105-glossary-variance-sep24-fxrxoasd.web.app

@parlough
Copy link
Member

parlough commented Oct 3, 2024

Sorry about the delay in reviewing this, I hope to get to it soon! Thanks again for working on this :D

Copy link
Member

@parlough parlough left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this @eernstg! I'm not very familiar with these concepts, so I naturally read it from a perspective of someone brand new to these terms. As a result, my questions and suggestions might be incorrect, so please feel free to push back on anything. At the very least, I hope my confusions and potential misunderstandings can at least help you identify where readers might go astray.

I left a few other small comments for the second pass so I can validate my understanding before leaving misleading suggestions. :)

Thanks again and sorry about the delay in reviewing this! I'm super excited to have something tangible in the docs to reference for these terms, and I can see it helping others as it was already insightful to me.

src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
Copy link
Member Author

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

Made several changes based on the review.

@parlough parlough self-requested a review October 8, 2024 03:34
Copy link
Member

@parlough parlough left a comment

Choose a reason for hiding this comment

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

Thanks so much for the answers to my questions and your edits @eernstg! This is looking great :D

I have a few follow-up comments for you to consider, but nothing blocking. I'm excited for us to have this useful reference!

src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
src/content/resources/glossary.md Outdated Show resolved Hide resolved
Copy link
Member Author

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

That's much better, thanks!

src/content/resources/glossary.md Outdated Show resolved Hide resolved
@eernstg
Copy link
Member Author

eernstg commented Oct 16, 2024

@parlough, I think this could be landed now. WDYT?

@parlough parlough merged commit 6ecfaea into main Oct 16, 2024
10 checks passed
@parlough parlough deleted the glossary_variance_sep24 branch October 16, 2024 19:35
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.

Covariant / Covariance definition
3 participants