-
Notifications
You must be signed in to change notification settings - Fork 3
chore: reporter code splitting #316
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
Conversation
|
Lets use util world instead helpers, it will keep naming consistent with ArgoCD project |
|
|
||
| if instanceNameIncludesNs(instanceName) { | ||
| return parseInstanceName(instanceName) | ||
| } else { |
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.
this is redundant
| namespace := parts[0] | ||
| app := parts[1] | ||
|
|
||
| return AppIdentity{ |
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.
i would return references, it is easier to operate with them
| @@ -0,0 +1,52 @@ | |||
| package reporter | |||
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.
i would move it to some util folder
| lastHistory := getLatestAppHistoryItem(a) | ||
|
|
||
| if lastHistory != nil { | ||
| id = lastHistory.ID |
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.
maybe better add return here, and not keep id as variable
| lastHistory := getLatestAppHistoryItem(a) | ||
|
|
||
| if lastHistory != nil { | ||
| revision = lastHistory.Revision |
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.
same here, just return and if it is not exists return a.Status.Sync.Revision
|
|
||
| func getOperationRevision(a *appv1.Application) string { | ||
| var revision string | ||
| if a != nil { |
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.
lets change to if a == nil { return "" } , and everything outside the block, such code will be easier to read
e614d06 to
7647baa
Compare
Checklist: