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

fix(refactor): refactor code #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix(refactor): refactor code #31

wants to merge 2 commits into from

Conversation

giangbui
Copy link
Contributor

No description provided.

@giangbui giangbui requested a review from Avantol13 July 30, 2018 13:19
@@ -314,7 +314,7 @@ def get_primary_service_account(self, proxy_group_id):
if service_account_id_for_user in service_account_emails:
primary_email = service_account_emails[service_account_id_for_user]

return self.get_service_account(primary_email)
return self.get_service_account(primary_email).json()
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are breaking changes. did you make sure you fixed every spot where these functions gets called in fence?

Copy link
Collaborator

Choose a reason for hiding this comment

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

this refactoring may be better off waiting until we've done more implementation, since it might be a headache to fix all the spots in fence that rely on this. I'd almost suggest we wait to do this cirrus cleanup/refactor until later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed all the code called to that function

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.

None yet

2 participants