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

feature: added golint in pipeline #1223

Merged
merged 28 commits into from
Feb 20, 2024
Merged

Conversation

officialasishkumar
Copy link
Contributor

@officialasishkumar officialasishkumar commented Dec 18, 2023

Related Issue

feature: add golint in pipeline

Closes: #1216

Describe the changes you've made

  • Added lint in .github/workflowto run only on the changes made in the PR.
  • Added configuration in .golangci.yml

Type of change

Feature

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, local variables)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Please let us know if any test cases are added

Please describe the tests(if any). Provide instructions how its affecting the coverage.

Describe if there is any unusual behaviour of your code(Write NA if there isn't)

NA

Checklist:

  • My code follows the style guidelines of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

Screenshots (if any)

Original Updated
original screenshot updated screenshot

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Copy link

sweep-ai bot commented Dec 18, 2023

Apply Sweep Rules to your PR?

  • Apply: All new business logic should have corresponding unit tests.
  • Apply: Refactor large functions to be more modular.
  • Apply: Add docstrings to all functions and file headers.

@officialasishkumar
Copy link
Contributor Author

@shivamsouravjha PTAL

@shivamsouravjha
Copy link
Contributor

seems to have some error as it doesn't run on this PR

@officialasishkumar
Copy link
Contributor Author

@shivamsouravjha,
image
It only checks the files that have changed in the PR. Since I never made any changes in the codebase apart from adding the Pipeline, so it doesn't run on the whole codebase.

@shivamsouravjha
Copy link
Contributor

to be clear the expectation is that once a PR is raise this should check for the code changes in the PR and be able to detect any changes that has lint issues

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@officialasishkumar
Copy link
Contributor Author

officialasishkumar commented Dec 26, 2023

@shivamsouravjha, Just for demo I ran the linter on the whole codebase. As you can see now the test is failing. Here is the log

@shivamsouravjha
Copy link
Contributor

Will it possible to not involve in previous errors ? as because of it folks won't be able to merge any PR.

@officialasishkumar
Copy link
Contributor Author

officialasishkumar commented Dec 27, 2023

Will it possible to not involve in previous errors ? as because of it folks won't be able to merge any PR.

@shivamsouravjha
Yeah, it's possible. We can do that by this line

# Optional: show only new issues if it's a pull request. The default value is `false`.
# only-new-issues: true

Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
@officialasishkumar
Copy link
Contributor Author

@shivamsouravjha PTAL

@shivamsouravjha
Copy link
Contributor

Tried running this in pipeline got this errorf https://github.com/keploy/samples-go/actions/runs/7538153673/job/20518222700?pr=91

@officialasishkumar
Copy link
Contributor Author

officialasishkumar commented Jan 16, 2024

@shivamsouravjha, Thanks for the reply.

The error only occurs for samples-go because of this reason. I think there will be no errors if the pipeline is ran in Keploy repo.

I've opened a PR for modified version of the golint pipeline for samples-go which is in draft. I will let you know once it is working fine.

@shivamsouravjha
Copy link
Contributor

@officialasishkumar I've added @PranshuSrivastava as reviewer as well.

@PranshuSrivastava PranshuSrivastava removed their request for review February 2, 2024 07:00
@PranshuSrivastava
Copy link
Member

@shivamsouravjha PTAL

@slayerjain
Copy link
Member

I think its good start

Copy link
Member

@slayerjain slayerjain left a comment

Choose a reason for hiding this comment

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

LGTM

@slayerjain slayerjain merged commit e117ea1 into keploy:main Feb 20, 2024
14 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature]: add golint in pipeline
5 participants