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(ci/lint): fix failing lint checks #212

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

pk-218
Copy link
Member

@pk-218 pk-218 commented Aug 30, 2022

Signed-off-by: pk-218 53428217+pk-218@users.noreply.github.com

Description

This PR fixes #203

Notes for Reviewers

Signed commits

  • Yes, I signed my commits.

Signed-off-by: pk-218 <53428217+pk-218@users.noreply.github.com>
@welcome
Copy link

welcome bot commented Aug 30, 2022

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

@theBeginner86
Copy link
Member

Thanks, @pk-218 for this PR.

This doesn't seem to entirely take care of them. Will you please see the lint check failures (here)?

@theBeginner86 theBeginner86 added area/ci Continuous integration | Build and release kind/chore Necessary task labels Sep 2, 2022
@pk-218
Copy link
Member Author

pk-218 commented Sep 3, 2022

That's strange. When I used act to run the CI locally, I found no issues.
Will look into it

@leecalcote
Copy link
Member

Maybe it has improved. I never really had any luck with act.

@pk-218
Copy link
Member Author

pk-218 commented Sep 3, 2022

If not act, how do you run GitHub Actions locally?

Signed-off-by: pk-218 <53428217+pk-218@users.noreply.github.com>
@theBeginner86
Copy link
Member

If not act, how do you run GitHub Actions locally?

I personally use golangci-lint, a command line tool for lint testing Golang code (check more here). One of the reason I use it is because it the exact same tool which we use in our testing scripts so it would do the exact same level of testing locally as done in the CI environment of layer5 projects.

Comment on lines +179 to +187
files := make([]fs.FileInfo, 0, len(entries))
for _, entry := range entries {
file, err := entry.Info()
if err != nil {
return err
}
files = append(files, file)
}

Copy link
Member

Choose a reason for hiding this comment

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

@pk-218 what's the need for this? Is this part of lint failures? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

The lint checks were failing here since ioutil.ReadDir is deprecated

As per the documentation, the replacement is os.ReadDir but there is a change in the method return value. Hence, to accommodate the changes, I had to add this code.

Copy link
Member

Choose a reason for hiding this comment

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

Ohh I see.

@pk-218
Copy link
Member Author

pk-218 commented Sep 4, 2022

Same, that's how I can catch and fix these failing lint checks xD
Was wondering what else Lee uses apart from act to run other GitHub Actions locally

Copy link
Member

@theBeginner86 theBeginner86 left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @pk-218 for looking through this.

@Revolyssup Revolyssup merged commit eda1471 into meshery:master Sep 8, 2022
@welcome
Copy link

welcome bot commented Sep 8, 2022

Thanks for your contribution to the Layer5 community! 🎉

Congrats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Continuous integration | Build and release kind/chore Necessary task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Golang Lint issues in MeshKit
4 participants