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: moved unwanted public functions from Helper.h (#766) #778

Merged

Conversation

Mehdibenhadjkhelifa
Copy link
Contributor

I have moved all the functions that aren't needed to be visible via the header file helper.h to a the AlgoHelper.h file under src/common while fixing all other cpp files dependent on those function also changed the cmake file under src/ to include the AlgoHelper.h file too. This was tested to be functional using the UT provided by the project ( faker-cxx-UT) all tests passed. Also used clang-format on most of the files (forgot some) that i have changed whilst working in this refactoring as prescribed in the CONTRIBUTING.md file. Hope this helped ! Cheers !

@Mehdibenhadjkhelifa Mehdibenhadjkhelifa changed the title Refactor:Moved unwanted public functions from Helper.h (#766) refactor:Moved unwanted public functions from Helper.h (#766) Jul 4, 2024
@Mehdibenhadjkhelifa Mehdibenhadjkhelifa changed the title refactor:Moved unwanted public functions from Helper.h (#766) refactor: moved unwanted public functions from Helper.h (#766) Jul 4, 2024
src/common/AlgoHelper.h Outdated Show resolved Hide resolved
@cieslarmichal
Copy link
Owner

Looks good, only minor comment :)

@cieslarmichal
Copy link
Owner

and resolve conflicts

@Mehdibenhadjkhelifa
Copy link
Contributor Author

Hello @cieslarmichal , i'm happy that you found my contribution useful but i wanted to know how that conflict happend? i didn't manually change the code and i made sure that i was up to date in my fork before commiting changes.I'm newish to open-source contribution so it may be something obvious that i'm missing that's why i wanted to ask. Also for the docs do you want me to make a new commit with the changes and push it or how do i follow up with your request exactly? Thanks for your time and patience !

@cieslarmichal
Copy link
Owner

Very good changes 👌 so I changed that file in my changes a moment ago so it has conflict now 😁

Copy link
Owner

@cieslarmichal cieslarmichal left a comment

Choose a reason for hiding this comment

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

🔥🔥🔥

@cieslarmichal cieslarmichal linked an issue Jul 4, 2024 that may be closed by this pull request
@cieslarmichal cieslarmichal merged commit cd3bc68 into cieslarmichal:main Jul 4, 2024
15 checks passed
@Mehdibenhadjkhelifa Mehdibenhadjkhelifa deleted the feature/move-internal branch July 4, 2024 18:10
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.

move internal helper functions to common files
2 participants