-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Reorganize Events
and EventSequence
code
#9306
Reorganize Events
and EventSequence
code
#9306
Conversation
I agree with merging the impl blocks and placing them next to the struct -- however I'd prefer for |
This reverts commit 7d30bb3.
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.
Thanks for taking my advice! This file is much easier to read without the impl block split for no reason.
Marking as ready for final review, since this is a trivial change.
Thank you for reviewing ^^ Part of the reason I split the two changes in two commits is so I can easily revert the second one as I knew it wouldn't necessarily win unanimous support. I'm aware the convention is to see how something is used first then how it's implemented, but I thought for this case since it's so small and doesn't really have a use in itself, and the thing that use it is pretty big, it could make sense to put it before, but yeah I think it doesn't matter and you're right. |
Example |
# Objective Make code relating to event more readable. Currently the `impl` block of `Events` is split in two, and the big part of its implementations are put at the end of the file, far from the definition of the `struct`. ## Solution - Move and merge the `impl` blocks of `Events` next to its definition. - Move the `EventSequence` definition and implementations before the `Events`, because they're pretty trivial and help understand how `Events` work, rather than being buried bellow `Events`. I separated those two steps in two commits to not be too confusing. I didn't modify any code of documentation. I want to do a second PR with such modifications after this one is merged.
Objective
Make code relating to event more readable.
Currently the
impl
block ofEvents
is split in two, and the big part of its implementations are put at the end of the file, far from the definition of thestruct
.Solution
impl
blocks ofEvents
next to its definition.EventSequence
definition and implementations before theEvents
, because they're pretty trivial and help understand howEvents
work, rather than being buried bellowEvents
.I separated those two steps in two commits to not be too confusing. I didn't modify any code of documentation. I want to do a second PR with such modifications after this one is merged.