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

Restructuring and more files #22

Merged
merged 10 commits into from
Oct 8, 2024
Merged

Restructuring and more files #22

merged 10 commits into from
Oct 8, 2024

Conversation

xFrednet
Copy link
Collaborator

@xFrednet xFrednet commented Oct 7, 2024

This PR required some logic changes. These changes should be clearly marked by the commits starting with Logic:

This refactoring added a new layer between the runtime and the objects, called the core. Which holds the prototypes and specific sub classes.

I also ran clang format as part of this PR.


All changes are hard to explain/list, might be best to clone the branch and look through the changes by browsing the files.

@xFrednet xFrednet requested a review from mjp41 October 7, 2024 14:49
Copy link
Collaborator

@mjp41 mjp41 left a comment

Choose a reason for hiding this comment

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

This looks great. Thanks for doing it.

My only real comment is I don't like the name env it makes me think of environment variables, and hence something external to the runtime.

I would prefer

  • Core, or
  • Base.

These both reflect that it is really essential to the runtime. .NET calls the equivalent the BCL (Base class library).

@xFrednet
Copy link
Collaborator Author

xFrednet commented Oct 8, 2024

Good suggestions, I'll change it to "core". "Environment" felt off to me, but I couldn't think of anything better at the time.

Once this is merged, would #20 also be ready, since this PR formatted all files?

@mjp41
Copy link
Collaborator

mjp41 commented Oct 8, 2024

Good suggestions, I'll change it to "core". "Environment" felt off to me, but I couldn't think of anything better at the time.

Once this is merged, would #20 also be ready, since this PR formatted all files?

Yeah, I think we can add a CI task to format the files and check for a diff. I can do that once you merge.

@xFrednet xFrednet merged commit 5cdc978 into main Oct 8, 2024
4 checks passed
@xFrednet xFrednet deleted the 0-cooking-lasagna branch October 8, 2024 08:49
@xFrednet
Copy link
Collaborator Author

xFrednet commented Oct 8, 2024

Perfect, then I've merged this now :D

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.

2 participants