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

Do not remove blank lines when building profile playbook #9809

Merged
merged 2 commits into from
Nov 14, 2022

Conversation

mildas
Copy link
Contributor

@mildas mildas commented Nov 11, 2022

@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@mildas
Copy link
Contributor Author

mildas commented Nov 11, 2022

@jan-cerny @matejak Do you remember why the removal of blank lines was introduced? Is there any risk when we don't remove them?

@jan-cerny
Copy link
Collaborator

@mildas I don't know exactly. I have found that this was introduced by #3688 and I dig in the history that there were multiple similar PRs at that time: #3687, #3685, #3684 etc. They are all related to Ansible roles, Ansible lint and Ansible Galaxy. My theory is that the blank lines cause some warning when the generated roles were uploaded in Ansible Galaxy.

But, we have a CI job here that builds the Ansible Roles and runs the Ansible Lint and its still green. What do you think about this CI job?

Side note: I discovered that this is the only call of the remove_multiple_blank_lines function so if we will remove it we can also remove the function.

@mildas
Copy link
Contributor Author

mildas commented Nov 14, 2022

@jan-cerny

My theory is that the blank lines cause some warning when the generated roles were uploaded in Ansible Galaxy.

There could be warning for multiple blank lines between Ansible tasks or between task parameters. But multiple blank lines in multi-line string shouldn't be a problem in my opinion as users might need to have such content there.

But, we have a CI job here that builds the Ansible Roles and runs the Ansible Lint and its still green. What do you think about this CI job?

The job looks useful to me and it doesn't skip any warning related to blank lines.

Side note: I discovered that this is the only call of the remove_multiple_blank_lines function so if we will remove it we can also remove the function.

Great catch! I've removed it in last commit

@codeclimate
Copy link

codeclimate bot commented Nov 14, 2022

Code Climate has analyzed commit edcae7c and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 47.2% (0.0% change).

View more on Code Climate.

@jan-cerny
Copy link
Collaborator

The CI fail in Fedora rawhide is unrelated to this PR, it's caused by libxml2 ABI break, see https://bugzilla.redhat.com/show_bug.cgi?id=2139546

@jan-cerny jan-cerny merged commit 4413901 into ComplianceAsCode:master Nov 14, 2022
@jan-cerny jan-cerny self-assigned this Nov 14, 2022
@jan-cerny jan-cerny added this to the 0.1.65 milestone Nov 14, 2022
@jan-cerny jan-cerny added the Infrastructure Our content build system label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Our content build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

audit_ospp_general fails after Ansible remediation because of missing blank lines
2 participants