search

christgau / wsdd

A Web Service Discovery host daemon.
MIT License
808 stars 97 forks source link

Add SysVinit start script #187

Closed Braintoe closed 7 months ago

Braintoe commented 7 months ago

Hello there,

I am a little unsure if this fits better here or in the MX Linux forum - but I guess here might be better since there is more than one distribution that might benefit from this... so just in case someone needs it: while I searched for the reason why wsdd did not load on my MX Linux 23, I created a new SysVinit script for wsdd based on the existing openrc script here and the old one that was used for MX Linux 21: wsdd.zip.

Maybe this could be added to the repository?

The script combines the nice workgroup extraction from smb.conf the openrc init script features with some basic logging functionality - wsdd does not detach itself which seems to require a start parameter for the daemon that makes stdout inaccessible, therefore the log I got before adding this was just empty.

Braintoe commented 7 months ago

Edit: I just noticed I uploaded the wrong version of the script - status does not work for that one. Here is the correct one: wsdd.zip

christgau commented 7 months ago

Would you mind to open up a pull request for this. That is a better place than an issue.

Braintoe commented 7 months ago

Would you mind to open up a pull request for this. That is a better place than an issue.

I have to admit I have no clue how to do that... I just did such a thing once, a couple of years ago, and I admit I do not remember how. Therefore sure, if you like, but I definitely need help on that topic (ideally in german...).

christgau commented 7 months ago

I see. This one may help https://docs.github.com/de/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

Braintoe commented 7 months ago

Thanks! Let me try... :-)

Braintoe commented 7 months ago

Okay, it seems I have succeeded...

JedMeister commented 7 months ago

To preface my comments here, I have nothing to do with this software (beyond using it).

I'm just a random person passing through. So whilst I like to think that my input will be useful, @christgau is the arbiter of how things work for this repo and perhaps I'm off the mark? So please do not feel any obligation to follow my suggestions (unless of course if @christgau explicitly agrees).

OTOH I doubt my suggestion(s) would be problematic, so if you think my suggestions make sense, then my guess is that you could apply them.

Finally, as a general rule most GitHub code repos will have a CONTRIBUTING` (or similar) file in the top level detailing the requirements. I note that it doesn't give any relevant guidance here specifically but it's good to double check IMO,

I'd recommend reopening this issue and editing the top post of your PR to include this (on it's own line - above or below - doesn't really matter which):

closes https://github.com/christgau/wsdd/issues/187

Then when your PR is merged by @christgau, this issue will be (automatically) closed. That way it's easy to be sure whether this "issue" is resolved in the master branch codebase or not.

Also, a couple of nitpicks (which @christgau may or may not agree with):

Braintoe commented 7 months ago

@JedMeister thanks for the remark. I reopened the issue and added the "closes" remark to the pull request as you suggested.

As a German, I am maybe more used to using capital letters than you. Apart from that, I named the files as they should be in MX Linux - which is "wsdd" in both cases, hence the subfolders. In the "/etc/openrc" folder here, the structure is set up in the very same way. But that is academical - any folder or file naming logic is up to @christgau since it is his very own project, therefore I will wait and change it to whatever he wants ;-)

christgau commented 7 months ago

First of all, thanks for the discussion here. @JedMeister I appreciate your input, as always

Here is advice for he PR:

  1. please use sysvinit as directory name
  2. the subfolder structure is ok, I was thinking of the openrc directory as well when reading the changes. IMO having the subfolders is fine. Could be it is a little inconsistent w.r.t. systemd, but it's ok 😉
  3. Change the commit message header for the PR (see CONTRIBUTING for details 😉)
Braintoe commented 7 months ago
  1. please use sysvinit as directory name

That one is clear, and after a while I also found how to do that on this website. Done!

  1. Change the commit message header for the PR (see CONTRIBUTING for details 😉)

While I have no clue for the reason (probably for automating... things... changelogs?), I hope I understood at least what you asked me to do... changed according to best guess. Please tell me if something is still wrong. I can safely say Github is not my world ;-)

christgau commented 7 months ago
  1. please use sysvinit as directory name

That one is clear, and after a while I also found how to do that on this website. Done!

Great. Thanks.

Just as a comment: Although the website may offer that service, you may also consider using git on the command line, if you want to. It would have been as easy as git mv etc/SysVinit etc/sysvinit

  1. Change the commit message header for the PR (see CONTRIBUTING for details 😉)

While I have no clue for the reason (probably for automating... things... changelogs?),

In principle it's about consistency in the repo's history and a question of style. Following a certain style of commit messages enables to quickly detect what a commit is about. It also allows for automation but that's currently not in use in that repo.

I hope I understood at least what you asked me to do... changed according to best guess. Please tell me if something is still wrong. I can safely say Github is not my world ;-)

You're welcome anyways. The primary reason I pointed to the contribution guide was the commit message, it should follow conventional commit message style. In this case, you add a new feature, so something like feat(etc): add SysVinit start script would be appropriate.

Braintoe commented 7 months ago

Thanks! Then I should have done the nright thing to that message.

Regarding git: yes, I have heard from that ans also found lots of corresponding notes when I searched for how to rename a folder. But I admit I don't really see a sense in installing something that I probably won't use much more than once. Even the content of the start script probably won't change much for the forseeable future. Those folder names probably won't ever :-)

christgau commented 7 months ago

"merged" with 628afd79334f4a25271e8d5264bf684190be5d17, although the PR #188 is "victim" of kind of an git accident 🙈

JedMeister commented 7 months ago

Regarding git: yes, I have heard from that ans also found lots of corresponding notes when I searched for how to rename a folder. But I admit I don't really see a sense in installing something that I probably won't use much more than once.

If you are using Linux (which your use of wsdd suggests you are) it should be as simple as:

apt install git

(Obviously the exact command will depend on your distro - but that will work for Debian/Ubuntu and derivatives).

Although git is a fairly common tool in Linux these days, so it's possible that you already have it installed...