#22 unixsockets

Closed
nobrowser wants to merge 3 commits from nobrowser/libmpdel:unixsockets into master
nobrowser commented 1 year ago

As dicussed over in mpdel/mpdel, this work provides a way to connect to MPD through a local (a.k.a. unix) socket.

As dicussed over in mpdel/mpdel, this work provides a way to connect to MPD through a local (a.k.a. unix) socket.
DamienCassou approved these changes 1 year ago
DamienCassou left a comment

Excellent job. I like the code and the decomposition into meaningful commits. Good job.

libmpdel.el Outdated
(defsubst libmpdel--open-stream ()
"Return the low level emacs stream to the mpd process."
(if (not (eq ?/ (aref libmpdel-hostname 0)))
DamienCassou commented 1 year ago

could you please extract this condition into a libmdel-hostname-for-unix-socket-p (or similar) function?

could you please extract this condition into a `libmdel-hostname-for-unix-socket-p` (or similar) function?
Poster
Owner

Thank you for your work!

Thank you for your work!
nobrowser commented 1 year ago
Poster

Per your request, I factored out the predicate that decides if the address is local.

Per your request, I factored out the predicate that decides if the address is local.
DamienCassou approved these changes 1 year ago
DamienCassou left a comment

Great job! Is there a reason why your PR is still marked WIP?

Could you please squash the latest commit into the commit introducing the check in the first place? This should result in 3 commits.

nobrowser changed title from WIP: unixsockets to unixsockets 1 year ago
nobrowser commented 1 year ago
Poster

Done?

Done?
Poster
Owner

Your commit history is messed up now:

  1. the first commit contains everything
  2. the first commit message is too long
  3. the second commit is empty
Your commit history is messed up now: 1. the first commit contains everything 2. the first commit message is too long 3. the second commit is empty
nobrowser commented 1 year ago
Poster

Your commit history is messed up now:

  1. the first commit contains everything
  2. the first commit message is too long
  3. the second commit is empty

That's because I tried to squash two of the commits, as you asked. rebase -i had a conflict which I resolved but apparently I did something wrong there. :-(

To tell the truth I have no energy to start over right now. How about squashing everything into a single commit?

> Your commit history is messed up now: > > 1. the first commit contains everything > 2. the first commit message is too long > 3. the second commit is empty That's because I tried to squash two of the commits, as you asked. `rebase -i` had a conflict which I resolved but apparently I did something wrong there. :-( To tell the truth I have no energy to start over right now. How about squashing everything into a single commit?
Poster
Owner

To tell the truth I have no energy to start over right now. How about squashing everything into a single commit?

fair enough. I changed the commits and pushed that to master. I also rephrased a bit a docstring.

Thank you again for your work!

> To tell the truth I have no energy to start over right now. How about squashing everything into a single commit? fair enough. I changed the commits and pushed that to `master`. I also rephrased a bit a docstring. Thank you again for your work!
DamienCassou closed this pull request 1 year ago
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.