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

refactor(profiles): rename Profile to ProfileManifest and update related functions #140

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Priyanshuthapliyal2005
Copy link

This pull request refactors the Profile struct to ProfileManifest across the codebase to resolve name collisions and improve code clarity.

Changes Made
Renamed the Profile struct to ProfileManifest in:
profiles.go
profiles_test.go
Related methods and variables.
Updated function signatures and internal references to align with the new struct name.
Ensured compatibility with all test cases, and verified that all tests pass successfully.

Why This Change Was Made
The previous implementation used the name Profile for both:
A variable representing eat.Profile (used as a ProfileID).
A struct that associates an eat.Profile with extensions.
This caused name collisions and confusion. Renaming the struct to ProfileManifest resolves these issues and aligns with naming conventions.

Testing
Ran the entire test suite using go test ./... and confirmed all tests passed.
Validated that no functionality was broken after the refactor.
Issue Fixed
This pull request fixes #135

Copy link
Contributor

@setrofim setrofim left a comment

Choose a reason for hiding this comment

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

Please also update example_profile_test.go with the name change (i.e. rename the local profile to profileManifest, etc).

corim/profiles.go Outdated Show resolved Hide resolved
@Priyanshuthapliyal2005 Priyanshuthapliyal2005 marked this pull request as draft January 8, 2025 17:40
@Priyanshuthapliyal2005 Priyanshuthapliyal2005 marked this pull request as ready for review January 8, 2025 17:56
@Priyanshuthapliyal2005
Copy link
Author

hi @setrofim , thank you for your feedback. I have rename the methods to GetProfileManifest to better reflect their functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoRIM Extensions: Consider a suitable name for Profile Structure
2 participants