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

Bugfix: typo in handler name and loop item handling in when statement #71

Merged

Conversation

MarcoV-git
Copy link
Contributor

  • Fixed typo in handler name
  • Added extra checks in using loop items in when statement

Tested in:
ansible [core 2.14.7]
config file = /Users/xxxxx/ansible.cfg
configured module search path = ['/Users/xxxxx/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
ansible python module location = /usr/local/lib/python3.11/site-packages/ansible
ansible collection location = /Users/xxxxx/.ansible/collections:/usr/share/ansible/collections
executable location = /usr/local/bin/ansible
python version = 3.11.4 (main, Jun 20 2023, 16:59:59) [Clang 14.0.3 (clang-1403.0.22.14.1)] (/usr/local/opt/python@3.11/bin/python3.11)
jinja version = 3.1.2
libyaml = True

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on opening your first pull request and thank you for taking the time to help improve Ansible-Lockdown!
Please join in the conversation happening on the Discord Server as well.

@uk-bolly
Copy link
Member

hi @MarcoV-git

Thank you for taking the time to investigate the issue and raise the PR.
I can see that we appear to have an issue with the pipeline that i will investigate.
If i could ask if you can ensure that the commits are signed off and gpg signature is attached to each commit.
The fixes you have suggested are great finds
the typo for the handler is not a problem.
Could i ask that the conditional for the fix in the 6.1. files are separated as per other controls for consistency.e.g.

when: 
    - item is defined 
    - item.stdout | length > 0

many thanks

uk-bolly

@MarcoV-git
Copy link
Contributor Author

Hi @uk-bolly ,

My bad! Will fix it tomorrow.

Cheers,
MV

@MarcoV-git
Copy link
Contributor Author

Updated to be compliant

@MarcoV-git MarcoV-git force-pushed the bugfix_loop_register_results branch from 0e6c7df to 8c32d14 Compare July 21, 2023 08:06
Copy link
Member

@uk-bolly uk-bolly left a comment

Choose a reason for hiding this comment

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

Great find and thank you for the PR

@MarcoV-git
Copy link
Contributor Author

MarcoV-git commented Jul 24, 2023

@uk-bolly ,

My git log shows that I have signed both my commits. I do see an unsigned commit from you:

commit 8921431 (upstream/devel, upstream/HEAD, origin/devel, devel)
Merge: 166e231 9709779
Author: uk-bolly mark.bollyuk@gmail.com
Date: Thu Jul 6 22:56:48 2023 +0100

Merge pull request #69 from lucab85/devel

Fix 2.2.14 evaluate dnsmasq instead of postfix

c59099 and others added 3 commits July 24, 2023 17:56
- Fixed typo in handler name

Signed-off-by: Marco V <marco@osp.nl>
Signed-off-by: Marco V <marco@osp.nl>
Signed-off-by: Marco V <marco@osp.nl>
@MarcoV-git MarcoV-git force-pushed the bugfix_loop_register_results branch from 68720bf to 69813b5 Compare July 24, 2023 15:59
@MarcoV-git
Copy link
Contributor Author

@uk-bolly Finally fixed the pull

@uk-bolly uk-bolly merged commit 059aa6f into ansible-lockdown:devel Jul 25, 2023
3 checks passed
@MarcoV-git MarcoV-git deleted the bugfix_loop_register_results branch August 1, 2023 07:11
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