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

Integrate idp-client and ueba endpoints to SDK #125

Conversation

GunnerUjjwol
Copy link
Contributor

No description provided.

@codeclimate
Copy link

codeclimate bot commented Nov 18, 2022

Code Climate has analyzed commit 84408d2 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.


// CreateIdForUebaScript create session ID for Ueba setup script
func (store *ConnectionManager) CreateIdForUebaScript() (map[string]string, error) {
sessionId := make(map[string]string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe replace the map with a generic created object response model. Same for all map[string]string usage below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You meant the struct to be model to be sth like this, right?:

type sessionID struct {
ID     string     `json:"id"
}

sure, I will change that.

I was not aware of the convention we follow here, so in the beginning, I weighed in between "using the map" or "using a struct model with a single field". In fact, I first created the struct model, but I ultimately got lured into using the map.
My kind of reasoning was,

  1. The return type has only one "key-value" pair, so I thought maybe a map could just fit the case.
  2. Since, this "struct" model will only be used once in the entire code. I didn't see the advantage we can get relating to reusability, so I kind of inclined towards the map.

But yes, I will change it.

api/connectionmanager/client.go Outdated Show resolved Hide resolved
api/connectionmanager/client.go Outdated Show resolved Hide resolved
api/connectionmanager/client.go Outdated Show resolved Hide resolved
@cuongssh cuongssh merged commit cd2f126 into SSHcom:master Nov 21, 2022
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