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

chore: minor clean-ups #536

Merged
merged 20 commits into from
Sep 14, 2023

Conversation

bartoszmajsak
Copy link
Contributor

@bartoszmajsak bartoszmajsak commented Sep 14, 2023

Description

While working on a Service Mesh component, I discovered several areas in the codebase that could be slightly improved. This pull request aims to address those issues for better code quality and maintainability.

Changes

  • Improved error handling by properly addressing previously ignored errors
  • Unified receiver names across funcs for all components
  • Updated deprecated methods such as AddToScheme and ioutil functions
  • Cleaned up the code by removing unneeded string conversions, redundant import aliases, and unnecessary variables in loops.
  • Improved error handling specifically when checking managed RHODS.
  • Wrapped existing errors rather than constructing new ones, for more context in error messages.
  • Fixed documentation comments to follow GoDoc conventions, including starting comments with the function name.
  • Removed an unused module and dead code
  • Fixed typos

These changes are proposed separately from the component work, so the final PR won't be that large, but also to speed up their review and potential inclusion into the main codebase.

How Has This Been Tested?

make test

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@bartoszmajsak bartoszmajsak requested review from zdtsw and VaishnaviHire and removed request for LaVLaS and VaishnaviHire September 14, 2023 13:42
pkg/deploy/deploy.go Outdated Show resolved Hide resolved
components/dashboard/dashboard.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@VaishnaviHire
Copy link
Member

Thanks @bartoszmajsak for these fixes! Changes look good to me

/lgtm

@openshift-ci openshift-ci bot removed the lgtm label Sep 14, 2023
@bartoszmajsak
Copy link
Contributor Author

@VaishnaviHire thanks, I added one additional thing while cleaning up error handling, please take a look when you'll find some time cdc4416 (#536)

Copy link
Member

@zdtsw zdtsw left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Sep 14, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VaishnaviHire, zdtsw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit efe23f7 into opendatahub-io:main Sep 14, 2023
@bartoszmajsak bartoszmajsak deleted the cleanups branch September 15, 2023 08:47
VaishnaviHire pushed a commit to VaishnaviHire/opendatahub-operator that referenced this pull request Sep 29, 2023
* fix: handles err when checking managed RHODS

* chore: fixes typos and godoc convention

* chore: removes redundant pattern

* chore: removes dead code

* chore(deps): removes unused module

* chore: fixes godoc to use documented function name first

* chore: unifies receiver names

* chore: wraps error instead of constructing new one

* fix: removes rendundant _ exp from for loop

* fix: removes redundant import alias

* fix: removes unneeded string conversion

* chore: switches file operations from depracated ioutil to os pkg

* chore: swaps deprecated AddToScheme with Install

* chore: handles ignored errors

* Revert "chore: wraps error instead of constructing new one"

This reverts commit da574c4.

* chore: wraps errors using %w directive

* chore: simplifies component error handling using multierr

* chore: cleans up imports

* chore: update docs in pkg/deploy/deploy.go

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

---------

Co-authored-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit efe23f7)
VaishnaviHire pushed a commit to VaishnaviHire/opendatahub-operator that referenced this pull request Sep 29, 2023
* fix: handles err when checking managed RHODS

* chore: fixes typos and godoc convention

* chore: removes redundant pattern

* chore: removes dead code

* chore(deps): removes unused module

* chore: fixes godoc to use documented function name first

* chore: unifies receiver names

* chore: wraps error instead of constructing new one

* fix: removes rendundant _ exp from for loop

* fix: removes redundant import alias

* fix: removes unneeded string conversion

* chore: switches file operations from depracated ioutil to os pkg

* chore: swaps deprecated AddToScheme with Install

* chore: handles ignored errors

* Revert "chore: wraps error instead of constructing new one"

This reverts commit da574c4.

* chore: wraps errors using %w directive

* chore: simplifies component error handling using multierr

* chore: cleans up imports

* chore: update docs in pkg/deploy/deploy.go

Co-authored-by: Wen Zhou <wenzhou@redhat.com>

---------

Co-authored-by: Wen Zhou <wenzhou@redhat.com>
(cherry picked from commit efe23f7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants