search

AllStarLink / app_rpt

Refactoring and upgrade of AllStarLink's app_rpt, etc.
3 stars 2 forks source link

simpleusb.conf: Some config option comments have typos #312

Closed InterLinked1 closed 2 months ago

InterLinked1 commented 2 months ago

It appears some of the documented config options have an apostrophe instead of a semicolon for beginning a comment, e.g.: https://github.com/AllStarLink/app_rpt/blob/master/configs/samples/simpleusb.conf.sample#L73

These were likely typos since they are next to each other on the keyboard.

This is an easy fix, I'm just making a note so it's not forgotten for when I or someone has time to work this.

Allan-N commented 2 months ago

I will note that the typos are in the .../configs/samples/simpleusb.conf.sample file (not .../configs/rpt/simpleusb.conf).

Why do we have/need sample conf files that are different from the ones that we install?

Hmmm... these ".sample" files only seem to come into play when one uses "make samples". Maybe we should nix the variants? or how about having rpt_install.sh copy the .../configs/rpt/... files into the asterisk configs/samples directory (with the .sample extension)?

InterLinked1 commented 2 months ago

I will note that the typos are in the .../configs/samples/simpleusb.conf.sample file (not .../configs/rpt/simpleusb.conf).

Why do we have/need sample conf files that are different from the ones that we install?

Hmmm... these ".sample" files only seem to come into play when one uses "make samples". Maybe we should nix the variants? or how about having rpt_install.sh copy the .../configs/rpt/... files into the asterisk configs/samples directory (with the .sample extension)?

From an Asterisk perspective, the .sample files are mandatory. The rpt ones are not since they are not samples, they are for ASL.

The rpt files could be moved into another repo at some point, in theory, but the samples are official and stay in the codebase.

Allan-N commented 2 months ago

From an Asterisk perspective, the .sample files are mandatory. The rpt ones are not since they are not samples, they are for ASL.

OK

The rpt files could be moved into another repo at some point, in theory, but the samples are official and stay in the codebase.

My main point is to suggest that the [installed] .conf file(s) be the same as the .conf.sample(s).

To follow the Asterisk guidelines it sounds like we should move the official files to .../configs/samples/*.conf.sample and ensure that our package build steps exec "make samples".

InterLinked1 commented 2 months ago

From an Asterisk perspective, the .sample files are mandatory. The rpt ones are not since they are not samples, they are for ASL.

OK

The rpt files could be moved into another repo at some point, in theory, but the samples are official and stay in the codebase.

My main point is to suggest that the [installed] .conf file(s) be the same as the .conf.sample(s).

To follow the Asterisk guidelines it sounds like we should move the official files to .../configs/samples/*.conf.sample and ensure that our package build steps exec "make samples".

The files are already in the right directory. If the contents of this repo were added to an Asterisk source tree, make samples would install those as sample configs in /etc/asterisk when running make samples.

I don't know what's desired with the ASL packages specifically - sample configs or the ASL ones - but I'll leave all that to the packagers. I think the goal was to make the rpt ones something that's easy for ASL users to run with.

Allan-N commented 2 months ago
  1. there are many .conf files in configs/rpt that do not have a corresponding sample in configs/samples.
  2. none of the configs/samples/*.conf.sample files that are present match the corresponding configs/rpt/*.conf files.
  3. we don't even have a configs/samples/rpt.conf.sample

Yes, we should fix the typos you highlighted but it seems like that we need to do more. Do we need another issue?

InterLinked1 commented 2 months ago
  1. there are many .conf files in configs/rpt that do not have a corresponding sample in configs/samples.

Yes, that's true... some of them are already upstream files e.g. logger.conf but some are not, e.g. beagle.conf

  1. none of the configs/samples/*.conf.sample files that are present match the corresponding configs/rpt/*.conf files.

They're not supposed to match, they serve different purposes.

  1. we don't even have a configs/samples/rpt.conf.sample

Ah, yes, that's a pretty big omission.

Yes, we should fix the typos you highlighted but it seems like that we need to do more. Do we need another issue?

Yeah, probably - thanks for pointing those out.

Allan-N commented 2 months ago
  1. none of the configs/samples/.conf.sample files that are present match the corresponding configs/rpt/.conf files.

They're not supposed to match, they serve different purposes.

Really? What is the purpose of each?

To me, I would think that either should provide [the admin] with the basis for a viable configuration. Right now, the files differ mostly because one file appears to have been updated and not the other.

InterLinked1 commented 2 months ago
  1. none of the configs/samples/.conf.sample files that are present match the corresponding configs/rpt/.conf files.

They're not supposed to match, they serve different purposes.

Really? What is the purpose of each?

To me, I would think that either should provide [the admin] with the basis for a viable configuration. Right now, the files differ mostly because one file appears to have been updated and not the other.

The sample file mainly exists to document all options and should be able to stand on its own and is what would be upstreamed into the Asterisk tree in the future. These are generic and don't contain anything ASL-specific.

The rpt ones are ASL-recommended configs as part of the "ASL bundle" as I like to think of it for people that want some kind of stock boilerplate configs they can run with. Anything ASL-specific would also go in these.