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

Refactor ImageStreamService loop to remove use of continue and go to statements #2531

Closed
4 tasks done
rohanKanojia opened this issue Jan 10, 2024 · 5 comments · Fixed by #2547
Closed
4 tasks done

Refactor ImageStreamService loop to remove use of continue and go to statements #2531

rohanKanojia opened this issue Jan 10, 2024 · 5 comments · Fixed by #2547
Assignees
@724thomas
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Milestone
1.16.0

Comments

@rohanKanojia
Copy link
Member

rohanKanojia commented Jan 10, 2024 •
edited by manusa
Loading

Component

JKube Kit

Task description

Description

This loop in ImageStreamService needs some minor refactoring. It uses multiple continue and even go-to statement. Perhaps it's a remnant of legacy code that was ported from Fabric8 Maven Plugin.

https://github.com/eclipse/jkube/blob/275dc11e9bd0a268aac94f112ac2c443960d957b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java#L178-L209

Refactor this loop not to use continue statements.

Expected Behavior

Loop should not contain any continue statements

Acceptance Criteria

  • Get rid of continue statements on line 190, 194, 198
  • Get rid of go to statement in nested loop on line 208
  • Move nested loop to a separate private method
  • ImageStreamServiceTest should pass even after refactoring code
@rohanKanojia rohanKanojia added the good first issue Good for newcomers label Jan 10, 2024
@manusa manusa added the help wanted Extra attention is needed label Jan 10, 2024
@724thomas
Copy link
Contributor

hi, can i pick this up?

724thomas added a commit to 724thomas/jkube that referenced this issue Jan 11, 2024
@724thomas
Refactor findTagSha method for clarity
857c9d6 
Improve readability and maintainability of findTagSha in ImageStreamService

- Flattened nested loop structures for better readability.
- Extracted complex logic into private helper methods.
- Replaced continue statements with conditional logic.
- Enhanced error handling for clearer exception messages.

This refactor is in line with our goal to simplify complex methods and
improve the overall code quality in the ImageStreamService class.

Resolves eclipse-jkube#2531
@724thomas 724thomas mentioned this issue Jan 11, 2024
Merged
17 tasks
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Jan 12, 2024
@rohanKanojia
test (jkube-kit/config/service) : Add test for ImageStreamService.fin…
8a1c90f 
…dTagSha

Related to eclipse-jkube#2531

+ Make retries and retry timeout configurable via new constructor
+ Add tests covering functionality for finding latest tag SHA in
  ImageStreamStatus received from OpenShift Api server.
+ Minor changes in old tests to use Kubernetes MockServer instead of
  mockito to mock OpenShiftClient

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
@rohanKanojia rohanKanojia mentioned this issue Jan 12, 2024
Merged
17 tasks
manusa pushed a commit that referenced this issue Jan 15, 2024
@rohanKanojia @manusa
test (jkube-kit/config/service) : Add test for ImageStreamService.fin…
c387c2b 
…dTagSha

Related to #2531

+ Make retries and retry timeout configurable via new constructor
+ Add tests covering functionality for finding latest tag SHA in
  ImageStreamStatus received from OpenShift Api server.
+ Minor changes in old tests to use Kubernetes MockServer instead of
  mockito to mock OpenShiftClient

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia pushed a commit to 724thomas/jkube that referenced this issue Jan 15, 2024
@724thomas @rohanKanojia
Refactor findTagSha method for clarity
9ebaaeb 
Improve readability and maintainability of findTagSha in ImageStreamService

- Flattened nested loop structures for better readability.
- Extracted complex logic into private helper methods.
- Replaced continue statements with conditional logic.
- Enhanced error handling for clearer exception messages.

This refactor is in line with our goal to simplify complex methods and
improve the overall code quality in the ImageStreamService class.

Resolves eclipse-jkube#2531
@rohanKanojia
Copy link
Member Author

@Swaminathan11 : There is already a PR to fix this issue #2547

You can try picking up some other good first issue that's not assigned to anyone.

@724thomas
Copy link
Contributor

Hello @rohanKanojia,

I'm writing to follow up on PR #2547, which has been approved but not yet merged. Could you please let me know if there's anything more required from my side to proceed with the merge? Any additional information or steps I need to be aware of would be very helpful.

Thanks for your assistance!

Best regards, Thomas

@rohanKanojia
Copy link
Member Author

@724thomas : We usually require 2 approvals in order to merge pull requests. I'll try to ping my colleague to review your pull request this week.

@724thomas
Copy link
Contributor

@724thomas : We usually require 2 approvals in order to merge pull requests. I'll try to ping my colleague to review your pull request this week.

Thank you for the update. Have a great week!

@manusa manusa added this to the 1.16.0 milestone Jan 23, 2024
manusa pushed a commit that referenced this issue Jan 23, 2024
@724thomas @manusa
Refactor findTagSha method for clarity
1807916 
Improve readability and maintainability of findTagSha in ImageStreamService

- Flattened nested loop structures for better readability.
- Extracted complex logic into private helper methods.
- Replaced continue statements with conditional logic.
- Enhanced error handling for clearer exception messages.

This refactor is in line with our goal to simplify complex methods and
improve the overall code quality in the ImageStreamService class.

Resolves #2531
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@724thomas 724thomas

Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Milestone
1.16.0
Development

Successfully merging a pull request may close this issue.

refactor: findTagSha method in ImageStreamService for improved readability 724thomas/jkube
3 participants
@manusa @rohanKanojia @724thomas