refactor: improve code readability with more descriptive naming (#133)
π Overview
This pull request addresses Issue #133 β "Improve Code Readability with More Descriptive Naming."
Shortened or unclear identifiers have been replaced with more descriptive and expressive ones across the backend and frontend codebases.
This enhances maintainability, self-documentation, and readability for all contributors.
π§© Backend Changes
- Replaced abbreviated variables with full descriptive names:
cfgβconfigurationsvcβinference_servicegvkβgroup_version_kindFILE_ABS_PATHβABSOLUTE_FILE_PATH
- Renamed helper functions:
inference_service_gvk()βinference_service_group_version_kind()_get_k8s_object()β_get_kubernetes_object()
- Unified constant naming:
K8S_*βKUBERNETES_*
- Added clearer docstrings and improved code comments.
π» Frontend Changes
- Improved naming consistency across Angular components:
currNamespaceβcurrentNamespacepollSubβpollingSubscriptionnsSubβnamespaceSubscriptionsvcβinferenceService
- Added
types/kubeflow.d.tsfor shared type definitions. - Enhanced dialog and configuration clarity:
configβdialogConfigresβdialogResponse
- Explicitly defined
name/valuepairs in chip components. - Updated
tsconfig.app.jsonpaths for type safety.
βοΈ GitHub Workflow Updates
- Improved readability and spacing in
.github/workflows/*.yamlfiles. - Standardized step comments and conditional blocks for better consistency.
π Benefits
- Improved readability and onboarding experience.
- More self-documenting codebase.
- Consistent naming conventions across frontend and backend.
- Stronger typing and maintainability.
β Checklist
- [x] Code compiles successfully
- [x] All tests and lint checks pass
- [x] Naming conventions verified
- [x] Linked to Issue #133
π Linked Issue
Closes #133
refactor: improve code readability with more descriptive naming (#133)
- Replaced abbreviated variables/functions with descriptive ones
- Unified naming for Kubernetes resource constants
- Improved clarity in API and route handlers
- Enhanced frontend naming conventions and dialog logic
- Added kubeflow.d.ts for shared type definitions
- Updated tsconfig paths and workflows for consistency
hi thanks for PR, please run prettier formatting and python black for linting so there are no effects in files which have no changes
hi thanks for PR, please run prettier formatting and python black for linting so there are no effects in files which have no changes β Ran Prettier and Black using pre-commit; all files are clean and formatted.
hi thanks for PR, please run prettier formatting and python black for linting so there are no effects in files which have no changes ran black and prettier , no unnessary whitespaces were found
Oh I like that PR :-)
Please ping me if its good to merge @LogicalGuy77
Oh I like that PR :-)
thanks , but there seems to be issue .Hey, I ran the frontend tests (npm run test:prod -- --browsers=ChromeHeadless --watch=false) and they all failed with
TypeError: Cannot redefine property: location
Looks like the test setup is trying to mock or redefine window.location, which isnβt allowed in ChromeHeadless. Is this something I should fix locally (like using a different test runner or disabling that mock), or is it an upstream issue with the test bootstrap that needs a patch on the repo side?
Hey, Glanced over your PR. Only name-related fields should be in full form; I donβt think there should be other changes.
Also, Iβd suggest making changes in smaller batches rather than pushing so many updates at once, large diffs like this can easily break parts of the app and make tests fail. Keep up the good work!
Hey, Glanced over your PR. Only name-related fields should be in full form; I donβt think there should be other changes.
Hello and thank you for the review. Regarding one point I disagree strongly. Also internal variable should be long pronounceable and expressive. "cfg" is wrong, "configuration" is right. Long pronounceable and expressive variable names and being able to write code as English text separates junior from senior engineers in hiring interviews. There are endless sources about the reasoning behind this. But it is definitively true.
So the idea from the first post is in general right:
- Replaced abbreviated variables with full descriptive names:
cfgβconfigurationsvcβinference_servicegvkβgroup_version_kindFILE_ABS_PATHβABSOLUTE_FILE_PATH
- Renamed helper functions:
inference_service_gvk()βinference_service_group_version_kind()_get_k8s_object()β_get_kubernetes_object()
- Unified constant naming:
K8S_*βKUBERNETES_*
- Added clearer docstrings and improved code comments.
@juliusvonkohout makes sense, I meant that tsconfig file should not be effected by these changes, besides that this looks great, I will run this locally and test it out., should be good to go then
What changes shall I do now @LogicalGuy77 @juliusvonkohout The issue lies in the test setup where window.location is being mocked or redefined. ChromeHeadless treats window.location as a read-only property, so attempting to overwrite it throws TypeError: Cannot redefine property: location.
What changes shall I do now @LogicalGuy77 @juliusvonkohout The issue lies in the test setup where window.location is being mocked or redefined. ChromeHeadless treats window.location as a read-only property, so attempting to overwrite it throws TypeError: Cannot redefine property: location.
we can first merge what works and is clarified and then you can raise a follow up PR for the remaining changes.
hi i removed the others changes and kept only these Replaced abbreviated variables with full descriptive names: cfg β configuration svc β inference_service gvk β group_version_kind FILE_ABS_PATH β ABSOLUTE_FILE_PATH Renamed helper functions: inference_service_gvk() β inference_service_group_version_kind() get_k8s_object() β get_kubernetes_object() Unified constant naming: K8S* β KUBERNETES* Added clearer docstrings and improved code comments.
Thank you, this looks a lot cleaner. Can you sign off your commit?