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

Fix some minor warnings in the matching package #4608

Merged
merged 4 commits into from
Jul 14, 2023

Conversation

MichaelSnowden
Copy link
Contributor

What changed?
I went through all the GoLand IDE inspection errors in the matching package, and I fixed the ones that seemed relevant. Most of them are little typos.

Why?
To clean things up a bit, make it easier to find actual errors from inspection, make sure these don't have to get included in other actual behavioral changes.

How did you test it?
I made sure not to include any behavioral changes, even small things like replacing err == with errors.Is.

Potential risks

Is hotfix candidate?

@MichaelSnowden MichaelSnowden requested a review from a team as a code owner July 10, 2023 18:38
func (db *taskQueueDB) GetUserData(
ctx context.Context,
) (*persistencespb.VersionedTaskQueueUserData, chan struct{}, error) {
func (db *taskQueueDB) GetUserData(context.Context) (*persistencespb.VersionedTaskQueueUserData, chan struct{}, error) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: I thought the original version is better and makes the line shorter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted except removing the param name

Comment on lines 238 to 242
type tasksBatch struct {
Tasks []*persistencespb.AllocatedTaskInfo
ReadLevel int64
IsReadBatchDone bool
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: getTasksBatchResponse maybe? read level and the done flag look like metadata to the returned batch of tasks.

do those fields have to be exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed and made them unexported

@MichaelSnowden MichaelSnowden enabled auto-merge (squash) July 13, 2023 23:16
@MichaelSnowden MichaelSnowden merged commit 7e2de69 into master Jul 14, 2023
@MichaelSnowden MichaelSnowden deleted the snowden/matching-warnings branch July 14, 2023 00:06
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