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.

Sign up for GitHub

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

Jump to bottom

fix: resolve lint errors #1682

Merged
merged 1 commit into from
May 2, 2023
Merged

fix: resolve lint errors #1682

merged 1 commit into from
May 2, 2023

Conversation

Chirag018
Copy link
Contributor

@Chirag018 Chirag018 commented May 2, 2023 •
edited
Loading

Closes #1671

@MSevey MSevey requested a review from a team May 2, 2023 15:59
@rootulp rootulp removed request for a team, adlerjohn, liamsi, MSevey and rach-id May 2, 2023 16:04
@rootulp
Copy link
Collaborator

rootulp commented May 2, 2023

@Chirag018 if you update the PR description to state: "Closes #1671" then the issue will automatically be closed when this PR merges. See https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword

@rootulp rootulp changed the title fix: lint issue resolved fix: resolve lint errors May 2, 2023
@rootulp
Copy link
Collaborator

rootulp commented May 2, 2023

Although before we close #1682 it would be nice to understand why the linter running on CI didn't catch these errors.

rootulp
rootulp approved these changes May 2, 2023
View reviewed changes
Copy link
Collaborator

@rootulp rootulp left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Testing

On this PR golangci-lint doesn't report any more errors

$ make lint
--> Running golangci-lint
--> Running markdownlint
--> Running hadolint
--> Running yamllint

@Chirag018
Copy link
Contributor Author

Although before we close #1682 it would be nice to understand why the linter running on CI didn't catch these errors.

These errors were not production-breaking issues.

@rootulp
Copy link
Collaborator

rootulp commented May 2, 2023

These errors were not production-breaking issues.

Where in the config are issues deemed production-breaking vs. non production-breaking?

@evan-forbes
Copy link
Member

Although before we close #1682 it would be nice to understand why the linter running on CI didn't catch these errors.

agree! it would be nice to enforce this in CI to be consistent instead of manually opening and checking PRs every now and then

@rootulp rootulp merged commit 82b22d4 into celestiaorg:main May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootulp rootulp rootulp approved these changes

@rach-id rach-id rach-id approved these changes

@evan-forbes evan-forbes evan-forbes approved these changes

Assignees

@Chirag018 Chirag018

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Identify why CI isn't failing for make lint errors
4 participants
@Chirag018 @rootulp @evan-forbes @rach-id