Skip to content

perf: added error handling missing code #2562

Merged
abeizn merged 7 commits into
apache:mainfrom
Pranshu-Raj:main
Aug 18, 2022
Merged

perf: added error handling missing code #2562
abeizn merged 7 commits into
apache:mainfrom
Pranshu-Raj:main

Conversation

@Pranshu-Raj
Copy link
Copy Markdown
Contributor

⚠️   Pre Checklist

Please complete ALL items in this checklist, and remove before submitting

  • I have read through the Contributing Documentation & PR Template
  • This PR is using a label (bug, feature etc.)
  • My code is has necessary documentation (if appropriate)
  • I have added any relevant tests
  • This section (⚠️   Pre Checklist) will be removed when submitting PR

Summary

Added error handling missing code in gitlab/api/connections.go (GET /plugins/gitlab/connections/:connectionId)
Added error handling missing code in gitlab/jenkins/connections.go (GET /plugins/jenkins/connections/:connectionId)
Added error handling missing code in gitlab/jira/connections.go (GET /plugins/jira/connections/:connectionId)
Added error handling missing code in gitlab/tapd/connections.go (GET /plugins/tapd/connections/:connectionId)

Does this close any open issues?

[Refactor][gitlab,jira,jenkins,tapd] Missing error handling #2561

Screenshots

Include any relevant screenshots here.

Other Information

Any other information that is important to this PR.

@mappjzc
Copy link
Copy Markdown
Contributor

mappjzc commented Jul 22, 2022

image
Whether error is empty or not, error is returned.
So we don't need to make "err ! = nil" judgment before this.

if err != nil {
return nil,err
}
return &core.ApiResourceOutput{Body: connection},nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please keep in line with other plugins, and missing space.

Copy link
Copy Markdown
Contributor Author

@Pranshu-Raj Pranshu-Raj Jul 23, 2022

Choose a reason for hiding this comment

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

okay that i did previously but why this lint commit-msg test is failing can u help me with that?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

both 165 and 167 need to add space, you added the space at 167 and missing 165, it maybe lint commit-msg test is failing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

try running make fmt. the go formatter should take care of these kind of things

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

seems like this is the only issue left for this PR, do you plan to work on it? @Pranshu-Raj

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sorry @klesh I am unable to resolve this issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No problem. I will try to fix the rest

Copy link
Copy Markdown
Contributor

@abeizn abeizn left a comment

Choose a reason for hiding this comment

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

LGTM

Added the error handling in GetConnection
added the error handling in GetConnection
added the error handling GetConnection
added the error handling GetConnection
edited the err handling err to nil
@abeizn abeizn merged commit 8007ff2 into apache:main Aug 18, 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.

5 participants