Skip to content

RFC: Rename neosmart_event_t to pevent_t#12

Open
mqudsi wants to merge 2 commits intomasterfrom
rename
Open

RFC: Rename neosmart_event_t to pevent_t#12
mqudsi wants to merge 2 commits intomasterfrom
rename

Conversation

@mqudsi
Copy link
Copy Markdown
Member

@mqudsi mqudsi commented Feb 19, 2021

I'm opening this as a request for comments in case anyone has any feedback before squash-merging the commits in this PR into the master branch.

The rationale for using pevent_t instead of the previous neosmart_event_t or the other option of pthread_event_t (which is probably the "classiest" option) is described in the commit messages:

The original name was an attempt to keep with the naming conventions
used by the pthreads library, along the lines of libname_objname_t.
However, it doesn't seem necessary or proper to do so if we are using
C++ with the code in the neosmart namespace.

It also doesn't seem correct to use pthread_event_t as the name if the
event object is not part of the pthread library proper, even if it is
built atop of it. Furthermore, there's no need for the name to be so
long and wordy if we have the advantage of C++ namespacing, since
there's no risk of a name collision.

The original name was an attempt to keep with the naming conventions
used by the pthreads library, along the lines of `libname_objname_t`.
However, it doesn't seem necessary or proper to do so if we are using
C++ with the code in the `neosmart` namespace.
It also doesn't seem correct to use `pthread_event_t` as the name if the
event object is not part of the `pthread` library proper, even if it is
built atop of it. Furthermore, there's no need for the name to be so
long and wordy if we have the advantage of C++ namespacing, since
there's no risk of a name collision.
@mqudsi mqudsi changed the title RFC: Rename event to pevent_t RFC: Rename neosmart_event_t to pevent_t Feb 19, 2021
@mqudsi mqudsi force-pushed the master branch 2 times, most recently from a60ccc6 to 9c22e4e Compare July 10, 2021 05:18
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.

1 participant