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

Lints #117

Merged
merged 3 commits into from
Jun 11, 2019
Merged

Lints #117

merged 3 commits into from
Jun 11, 2019

Conversation

cep21
Copy link
Contributor

@cep21 cep21 commented Jun 4, 2019

Fix a few lints that are somewhat easier to think about.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Fix a few lints that are somewhat easier to think about.
"testing"

"github.com/stretchr/testify/assert"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cep21
Copy link
Contributor Author

cep21 commented Jun 5, 2019

I think there's a bug in the code for SQL causing this test to fail.

		if strings.Contains(u.Path, "@") {
			u, _ = url.Parse(fmt.Sprintf("%s//%s%%2F%s", u.Scheme, u.Host, u.Path[1:]))
		}

		// Strip password from user:password pair in address
		if u.User != nil {

The error return is ignored, which is resetting u to nil and causing the next u.User to nil panic.

@luluzhao
Copy link
Contributor

luluzhao commented Jun 5, 2019

@cep21 you are right. We should catch the error.

@cep21 cep21 closed this Jun 11, 2019
@cep21 cep21 deleted the fix_lints branch June 11, 2019 00:29
@luluzhao
Copy link
Contributor

@cep21 , I am wondering why we close this PR?

@cep21 cep21 restored the fix_lints branch June 11, 2019 01:17
@cep21 cep21 reopened this Jun 11, 2019
@cep21
Copy link
Contributor Author

cep21 commented Jun 11, 2019

Oh that was a mistake. I deleted the branch on my end and I think github closed the PR :/ . I reopened the PR

@cep21 cep21 closed this Jun 11, 2019
@cep21 cep21 reopened this Jun 11, 2019
@luluzhao luluzhao merged commit 3565e35 into aws:master Jun 11, 2019
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.

2 participants