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

Maintainability #12

Merged
merged 35 commits into from
Oct 12, 2023
Merged

Maintainability #12

merged 35 commits into from
Oct 12, 2023

Conversation

carlocorradini
Copy link
Collaborator

@carlocorradini carlocorradini commented Oct 8, 2023

Complete rewrite with new structure, features and more...
I've set minimum CMake version to 3.14 (following CPM) but should be decreased.
@karljj1 Let me know what you think 🥳🤯

NOTE: Currently this is a draft (unfinished)

Fix #8
Fix #9
Fix #13

@karljj1
Copy link
Owner

karljj1 commented Oct 9, 2023

Looks good. Are these just changes to the structure or are there also code changes? Github was struggling to show the PR so I couldn't review it all.

@carlocorradini
Copy link
Collaborator Author

The code is the same (it has only been formatted by clang-format).
The only changes I made are in KDefines.hpp but it should be refactored since we will use GenerateExportHeader. Therefore the custom DLL export is deprecated/useless.
For the other changes etc... what do you think?

@karljj1
Copy link
Owner

karljj1 commented Oct 9, 2023

The code is the same (it has only been formatted by clang-format). The only changes I made are in KDefines.hpp but it should be refactored since we will use GenerateExportHeader. Therefore the custom DLL export is deprecated/useless. For the other changes etc... what do you think?

Oh very nice. GenerateExportHeader looks great :)
Im happy with the changes.

@carlocorradini
Copy link
Collaborator Author

@karljj1 Note that you can edit (create commits and push them) to my branch since I've allowed edits from maintainers

@carlocorradini
Copy link
Collaborator Author

@karljj1 I think we have a pretty good starting point now.
What do you think?
Note that the CI/CD has been improved.
Added GenerateExportHeader.
You can release a new version of KDIS automatically by pushing a tag (SEMVER compatible). E.g.: v3.0.0

@karljj1 karljj1 marked this pull request as ready for review October 11, 2023 10:00
@karljj1
Copy link
Owner

karljj1 commented Oct 11, 2023

Looks great! Im happy to merge this in if you are. Thanks for all the hard work :)

@karljj1 karljj1 self-requested a review October 11, 2023 10:01
@carlocorradini
Copy link
Collaborator Author

@karljj1 I think it's better if you release version 2.10.0 with the old code and structure (can you update the CHANGELOG.md file in my branch please).
Then, merge the PR (note that it's not finished yet) and update the CHANGELOG.md file in unreleased version (this will be 3.0.0).

@carlocorradini
Copy link
Collaborator Author

Nice 🥳🤯

@karljj1 karljj1 merged commit 3139f69 into karljj1:master Oct 12, 2023
@karljj1 karljj1 deleted the new branch October 12, 2023 15:16
@karljj1
Copy link
Owner

karljj1 commented Oct 12, 2023

Thanks for the PR! Really appreciate all the work you did,

@carlocorradini
Copy link
Collaborator Author

Wow 😳 Thanks
The repo looks really good now 🥳
We need to update all the code and more 🥳👍

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.

Convert changelog into markdown. chore: add clang-tidy chore: add .clang-format
2 participants