-
Notifications
You must be signed in to change notification settings - Fork 35
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
Update to builder v3 #112
Update to builder v3 #112
Conversation
46d34e7
to
030e09f
Compare
bd4e07b
to
0e65c96
Compare
0e65c96
to
78d2d44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, a lot simpler actually which is nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to fix the tests
74611f7
to
d998d8c
Compare
d998d8c
to
86d52b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's still failing tests does it require a merge before that will pass now? @sean-m-sullivan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM hopefully we can fix the test soon
No clue on if it will magically work after merge, I've tried multiple things and am exasperated at this point. |
What does this PR do?
Complete rewrite of the builder role, venv role update to match the new strucuture to come.
How should this be tested?
playbooks are provided for testing, I've tested it with multiple options getting the jinja correct,