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

Enum docs patch #26394

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

astatide
Copy link
Contributor

A quick PR to replace Kissinger with Plato.

@astatide astatide force-pushed the astatide/enum_docs_patch branch from 31a6c91 to 616216e Compare December 12, 2024 00:29
@cassella
Copy link
Contributor

FYI, the Chapel team generally suggests tagging a team member when opening a PR to be sure someone looks at it.

writeln("there's too much fraternizing with the enemy."); }
when statesman.Plato do
{ write("I only wish that wisdom were the kind of thing that flowed...");
writeln("... from the vessel that was full to the one that was empty."); }
Copy link
Contributor

@DanilaFe DanilaFe Dec 15, 2024

Choose a reason for hiding this comment

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

Thank you for your patch! I think you might need to update the expected output of the test, as well. See like 657 specifically. Please feel free to ping me when the PR is ready for another look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I had totally missed that! I've gone ahead and updated it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, I was double checking the attribution of the quote, and it looks like it ought to be attributed to Socrates? This Wikiquote page suggests that Plato wrote down the saying in the PR's diff as having been said by Socrates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, damn! I can't quite remember the source (I found a good site resource for it but the attribution format was a little strange), but it looks like you're right. Lemme try and find another one since Socrates is already in there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, wait; it's Aristotle that's already in here, not Socrates. Should I just update it to Socrates, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed! Also, excellent catch! Thank you for double checking!

@astatide astatide force-pushed the astatide/enum_docs_patch branch from f6e3b60 to cd896de Compare December 17, 2024 18:32
@astatide astatide requested a review from DanilaFe December 17, 2024 18:34
@astatide astatide force-pushed the astatide/enum_docs_patch branch from 4bddbc7 to f2b36a6 Compare December 23, 2024 21:04
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