Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(network_manager_networks): set tpldir in template #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alxwr
Copy link
Member

@alxwr alxwr commented Jul 22, 2022

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

saltstack-formulas/mysql-formula#187

Describe the changes you're proposing

Set tpldir in the context of the template.

Pillar / config required to test the proposed changes

None.

Debug log showing how the proposed changes work

host:
----------
          ID: /etc/NetworkManager/system-connections/udp_lan
    Function: file.managed
      Result: True
     Comment: File /etc/NetworkManager/system-connections/udp_lan updated
     Started: 00:31:29.634922
    Duration: 1902.506 ms
     Changes:   
              ----------
              diff:
                  New file
----------
          ID: network_manager_connection_reload
    Function: cmd.run
        Name: /usr/bin/nmcli connection reload
      Result: True
     Comment: Command "/usr/bin/nmcli connection reload" run
     Started: 00:31:47.269337
    Duration: 122.398 ms
     Changes:   
              ----------
              pid:
                  105712
              retcode:
                  0
              stderr:
              stdout:

Summary for host
-------------
Succeeded: 21 (changed=2)
Failed:     0
-------------
Total states run:     21
Total run time:   24.906 s

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

I didn't write tests, because this SLS configures the network manager, not OpenVPN itself. (I was reluctant to add another dependency to the existing test suite.)

@alxwr alxwr requested a review from dafyddj as a code owner July 22, 2022 22:36
@alxwr alxwr self-assigned this Jul 22, 2022
@alxwr
Copy link
Member Author

alxwr commented Aug 21, 2023

@daks @myii @javierbertoli

(Sorry for pulling you into my struggle. :-) But from your recent activity I derived that you are among the most active within saltstack-formulas.)

I have a quick question regarding CI: is there any documentation on how this is supposed to work? I don't have any idea where to start without jeopardizing previous work.
(I fully agree that CI is a valuable tool, but am I supposed to fix CI before I can merge a PR?)

@daks
Copy link
Member

daks commented Oct 25, 2023

@alxwr no problem :) FYI I'm not using salt anymore since several months and don't have a lot of time to invest in formulas management.

About CI: I don't think there is any documentation, and it should, because setup has changed several times (software used, provider...) and only a few maintainers can keep informed about those. The general formulas management (github accounts, ssf-formula...) feels under-documented for me amd complicates new contributors integration.

About fixing CI: I tend to think it should be fixed but it's not always possible. The ideal situation would be that CI is not broken and the minimal requirement for a PR is: do not break it! :)

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.

2 participants