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

ci: add linter for ansible playbook and disable dependentbot #31

Closed
wants to merge 1 commit into from

Conversation

kezhenxu94
Copy link
Member

No description provided.

@wu-sheng
Copy link
Member

CI somehow fails. Please check.

@wu-sheng wu-sheng added this to the 0.1.0 milestone Oct 26, 2023
@@ -15,19 +15,19 @@

---
- name: Install Java 11 on RHEL-based systems
package:
ansible.builtin.package:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not use the ansible.builtin.. I know this is being explicit instead of implicit. However, this is inconsistent with all our other playbooks. Also, this has no impact on the functioning of our playbooks. Let me know of you think otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's not use the ansible.builtin.. I know this is being explicit instead of implicit. However, this is inconsistent with all our other playbooks. Also, this has no impact on the functioning of our playbooks. Let me know of you think otherwise.

Thanks for the comment, the purpose of this pull request is to unify all the module names by using full-qualified names, and should be fixed in all playbooks in this repo, so if there is anything missing then I shall fix them all in this PR.

The main purpose is to add a linter to remind simple errors that might be introduced, and the default rules of the linter complains the builtin modules should be explicitly prefixed with ansible.builtin., do you prefer we skip this rule? I can add the rule to the skip list so we don't report this kind of errors in the linter

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I understand, if the linter suggests we must be explicit then let's be explicit. Also, I think the linter would work on PRs as we make changes to files. So let us not touch all the files and make changes rather with time as we make changes to playbooks, the linter will pop up and state modules explicitly. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, let me close the PR for now

@kezhenxu94 kezhenxu94 closed this Oct 31, 2023
@kezhenxu94 kezhenxu94 deleted the lint branch October 31, 2023 01:54
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