Skip to content

Add fmt, vet, staticcheck to CI and lint accordingly#89

Merged
peterbourgon merged 7 commits intooklog:mainfrom
craigpastro:typos
Nov 8, 2022
Merged

Add fmt, vet, staticcheck to CI and lint accordingly#89
peterbourgon merged 7 commits intooklog:mainfrom
craigpastro:typos

Conversation

@craigpastro
Copy link
Copy Markdown
Contributor

@craigpastro craigpastro commented Oct 24, 2022

Fix some small typos in the comments.

By the way, it also seems that ulid.go is not properly gofmted. For example, when I run it I get a bunch of stuff that looks like

-//    // Example usage.
-//    db.Exec("...", stringValuer(id))
+//	// Example usage.
+//	db.Exec("...", stringValuer(id))

in the diff. Would you like me to push those changes in this PR?

@peterbourgon
Copy link
Copy Markdown
Member

Sure! Thanks. The file is extremely organically grown :)

@craigpastro
Copy link
Copy Markdown
Contributor Author

No worries! I added a lint step to the ci. If you don't like/need that I can revert the commit.

@craigpastro craigpastro changed the title typos: "an ulid" --> "a ulid" Add lint step to CI and lint the code Oct 24, 2022
Comment on lines +11 to +12
with:
go-version-file: "./go.mod"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is necessary?

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.

Probably not. I'll remove it. Thanks!

Comment on lines +13 to +16
- name: Lint
uses: golangci/golangci-lint-action@v3
with:
version: latest
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not a fan of this one, let's do go fmt, go vet, and staticcheck.

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.

Okay. Done. Thanks!

ulid_test.go Outdated
Comment on lines +607 to +609
u0 := ulid.MustNew(t0, safe)
u1 := ulid.MustNew(t0, safe)
for j := 0; j < 1024; j++ {
u0, u1 = u1, ulid.MustNew(t0, safe)
u0, u1 := u1, ulid.MustNew(t0, safe)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🙅

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.

golangci-lint didn't like that. Reverted. Thanks!

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.

Oops. staticcheck also doesn’t like it.

@craigpastro craigpastro changed the title Add lint step to CI and lint the code Add fmt, vet, staticcheck to CI and lint accordingly Oct 24, 2022
@peterbourgon
Copy link
Copy Markdown
Member

OK, so maybe

u0 := ulid.MustNew(t0, safe)
u1 := u0
for j := 0; j < 1024; j++ {
	u0, u1 = u1, ulid.MustNew(t0, safe)
	...

?

@peterbourgon
Copy link
Copy Markdown
Member

Or if that doesn't work

var (
	u0 = ulid.MustNew(t0, safe)
	u1 ulid.ULID
)
...

@peterbourgon peterbourgon merged commit ab86345 into oklog:main Nov 8, 2022
@craigpastro craigpastro deleted the typos branch November 8, 2022 22:46
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