Skip to content

Add snac configuration file with commented out list of modules and update Makefile for installation#75

Closed
texasaggie97 wants to merge 4 commits intoni:masterfrom
texasaggie97:dev/texasaggie97/add-snac-conf-file
Closed

Add snac configuration file with commented out list of modules and update Makefile for installation#75
texasaggie97 wants to merge 4 commits intoni:masterfrom
texasaggie97:dev/texasaggie97/add-snac-conf-file

Conversation

@texasaggie97
Copy link
Copy Markdown
Collaborator

@texasaggie97 texasaggie97 commented Sep 26, 2025

Summary of Changes

  • Add default snac.conf
    • Contains empty [modules] section
    • Contains commented out list of all modules
  • Update Makefile to install to correct location

Justification

AB#3335800

Testing

Built locally and verified file in correct location

Procedure

#
# Example: To disable a module, uncomment the line and set to 'disabled'.
#
# ntp = disabled # NTP time synchronization
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment at the beginning is hard to notice. Since you already tell them above to set it to disabled, why not have this uncommented and set to enabled? That way the change the user must do is simpler.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to imply that you have to explicitly enable each module. If I put all of them enabled, that is what it will imply. At least that is how I would interpret it.

Plus I thought most conf files have the opposite of the default but commented out so that all you do is remove the '#'.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I didn't make my "not an expert" status explicit. If that's how most config files in Linux do it, by all means.

Copy link
Copy Markdown
Collaborator

@amstewart amstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, now that I'm looking at the diff, I've convinced myself / remembered that there is a more correct way to distribute this package.

This project's dist package operations (eg. our Makefile) shouldn't be installing things to the sysconf (/etc) locations unless we have to (as in the case of the wireguard configuration.) Instead, the Makefile install and uninstall should place the snac.conf file into the architecture-independent files location, eg.

$(DESTDIR)$(docdir)/$(PACKAGE)/snac.conf.example (installed with mode 0444)

And then, in meta-nilrt, the recipe file should be the copied into the sysconf location with the admins given write-access.

texasaggie97 and others added 4 commits September 29, 2025 10:08
structure and enable module control via config file

Signed-off-by: Mark Silva <mark.silva@emerson.com>
Signed-off-by: Mark Silva <mark.silva@emerson.com>
Co-authored-by: Dan Mondrik <dan.mondrik@ni.com>
Signed-off-by: Mark Silva <103217450+texasaggie97@users.noreply.github.com>
…date Makefile for installation

Signed-off-by: Mark Silva <mark.silva@emerson.com>

Review comment - makefile

Review comment - conf
@texasaggie97 texasaggie97 force-pushed the dev/texasaggie97/add-snac-conf-file branch from be9aa13 to 245fa64 Compare September 29, 2025 14:12
@texasaggie97
Copy link
Copy Markdown
Collaborator Author

I had to fix merge issues when rebasing to combine the requested changes. This caused many additional and unwanted changed to be added. I am abandoning this and starting over.

@texasaggie97 texasaggie97 deleted the dev/texasaggie97/add-snac-conf-file branch September 29, 2025 14:19
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.

3 participants