8000 Refactor & clean-up & tidy-up the ansible roles generator by isimluk · Pull Request #5741 · ComplianceAsCode/content · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactor & clean-up & tidy-up the ansible roles generator #5741

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 31 commits into from
May 6, 2020
Merged

Refactor & clean-up & tidy-up the ansible roles generator #5741

merged 31 commits into from
May 6, 2020

Conversation

isimluk
Copy link
Member
@isimluk isimluk commented May 6, 2020

These changes have no functional effect. It is only rewiring of the script to be more easily comprehended by humans.

  • split monster spaghetti class to two shorter ones, split is done to avoid feature creep, generation of ansible role is separated from github update logic
  • make code more declarative and thus easier to read and assess what is what

isimluk added 14 commits May 6, 2020 14:29
@isimluk
Refactor: Make _update_content_if_needed generic and re-usable
0b494d5
@isimluk
Refactor: Replace _update_vars_contetn_if_needed by generic method
6a37cd4
@isimluk
Refactor: replace _update_readme_content_if_needed by generic method
88f7eaf
@isimluk
Refactor: replace _update_meta_content_if_needed by generic method
085cc10
@isimluk
Get a rid of repo_status variable, it is no longer needed
95955bd
@isimluk
Refactor: replace add_task_variables_to_default_variables_if_needed b…
182585a 
…y generic method
@isimluk
Refactor: Consolidate Role class initialization
0b23e94
@isimluk
Refactor: Consolidate description handling to a single place
46dde11
@isimluk
Refactor: Extract class PlaybookToRoleConverter
0dc38a1 
This is part 1 of multipart effort to clean-up the logic in the Role class.
We are trying to dissolve github handling and role composition responsibilities
to the two separate classes.
@isimluk
Remove overabundant variable, not used
c539995
@isimluk
Refactor: Move defaults variable handling up to the converter class
9ea2f23
@isimluk
Refactor: simplify logic so that my eyes stop bleeding
d8f1b25
@isimluk
Refactor: Move task_local_content down to the ansible converter
f5d1619
@isimluk
Refactor: Move all the other post-processing down to the convertor class
60eaffa
@pep8speaks
Copy link
pep8speaks commented May 6, 2020 •
edited
Loading

Hello @isimluk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-05-06 12:44:34 UTC

@isimluk isimluk changed the title Refactor & clean-up & tidy-up the Refactor & clean-up & tidy-up the ansible roles generator May 6, 2020
@isimluk isimluk requested a review from redhatrises May 6, 2020 12:34
@isimluk isimluk force-pushed the ansibl-validations branch 2 times, most recently from 04be09e to 308b483 Compare May 6, 2020 12:40
@isimluk isimluk force-pushed the ansibl-validations branch from 308b483 to a976db7 Compare May 6, 2020 12:42
@isimluk isimluk force-pushed the ansibl-validations branch from a976db7 to 78c2057 Compare May 6, 2020 12:44
@mildas
Copy link
Contributor
mildas commented May 6, 2020

Changes identified:
Others:
 Python abstract syntax tree change found in utils/upload_ansible_roles_to_galaxy.py. - ctest will be selected.

Recommended tests to execute:
 (cd build && ctest -j4)

@jan-cerny
Copy link
Collaborator

@mildas I think that the suggestion of your bot isn't applicable in this case because this script isn't tested by ctest.

@redhatrises redhatrises added this to the 0.1.50 milestone May 6, 2020
@redhatrises
Copy link
Contributor

Nice! LGTM

@redhatrises redhatrises merged commit b660d81 into ComplianceAsCode:master May 6, 2020
@vojtapolasek vojtapolasek modified the milestones: 0.1.50, 0.1.51 May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@redhatrises redhatrises Awaiting requested review from redhatrises

Assignees

@redhatrises redhatrises

Labels
None yet
Projects
None yet
Milestone
0.1.51
Development

Successfully merging this pull request may close these issues.
6 participants
@isimluk @pep8speaks @mildas @jan-cerny @redhatrises @vojtapolasek
0