-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
WIP: Stylistic and syntactic updates to the manual #21456
Conversation
As long as what you've done so far is correct, this could just be done in a series of smaller PRs and just merge each one when it's done. |
That's an important question. 😉 Want to give it a review?
That's fine with me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, definitely all improvements.
@@ -123,7 +123,7 @@ Types such as `Union{Function,AbstractString}` are often a sign that some design | |||
When creating a type such as: | |||
|
|||
```julia | |||
type MyType | |||
struct MyType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this note relevant for immutable types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't it be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could only instantiate a value with one of the union members, and not change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, though I don't see that as being a problem. I can just make it mutable struct
if you think that's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's highlighting a problem, it should be doing so in a way that's representative of when it's actually a problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean. Could you clarify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tkelman: it would be easier if you just made a PR to fix your concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if I were at a computer right now, yes. mutable struct
would be better here since that's the situation where union typed fields is more likely to be a problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I'll add that to my other PR then. Thanks for clarifying.
Not ready to be merged yet, still a WIP.
My goal here is to make the stylistic conventions in the manual consistent, including (but not limited to) migrating to
where
andstruct
syntax as well as using the standard 4 spaces for indentation everywhere.