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

contributors: describe how to identify the locations of Kindling code #243

Open
marceline-cramer opened this issue Nov 24, 2023 · 4 comments
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@marceline-cramer
Copy link
Collaborator

Right now, we use identifiers in our commits, issues, and PRs for the locations in the codebase that each of those affect. For example:

guest: add export_metadata macro

...is the message of a commit that touches hearth-guest. This is really clean and makes it really easy to compartmentalize what code changes, bugs, and ideas affect which specific parts of the repository.

However, now that Kindling is part of the codebase, what we've been doing so far is this:

kindling-panic-test: add export_metadata

Not as clean and not as readable. panic-test may also be a relatively short name for a service. Cutting the kindling- prefix out removes the context as to whether panic-test is a guest- or host-side crate.

We also need to consider that we have a plugin named hearth-init AND a Kindling crate named kindling-init, which implements the init system. Should one be renamed? Perhaps the hooks system that init implements could either be phased out or moved into hearth-runtime::utils.

Could we find an effective location scheme to use for Kindling crates that integrate into our existing location scheme? Should we reconsider this system altogether? Can we rely on understanding of the repository to provide context for commit messages and so on that do not have this information data?

I dislike Conventional Commits because it's always been my opinion that is much higher priority to describe the location of code changes than whether a code change is a bugfix or an addition.

This topic is not one that I can navigate alone. I'd like to discuss our options and form a consensus on the best way forward. Once we find one, we should describe it in the CONTRIBUTORS.md doc.

This is also really relevant to the topic of Kindling code sharing so I've included it in the "Kindling common code" milestone.

@marceline-cramer marceline-cramer added documentation Improvements or additions to documentation question Further information is requested labels Nov 24, 2023
@marceline-cramer marceline-cramer added this to the Kindling common code milestone Nov 24, 2023
@airidaceae
Copy link
Collaborator

I agree a lot that the location of code changes is really important and I think that us doing that has been one of the reasons that the commit log is so legible.
Perhaps we could have something like a single letter identifier to say whether its guest or host
h-init: update flue version
g-init: implement hot reload
Or just a K to mark it as kindling
K-init: implement hot reload

Im not sold on this exact scheme but I think that a single letter could be better than a whole thing like kindling

K: panic-test: add export_metadata
The double colons here feel confusing.

I dont think that we should necessarily count on never having an overlap between host and guest crates. Our system should try to properly account for that.

As for longer service names, it may be a good idea to shorter them
K-panel-manager: properly export caps
K-panels: properly export caps
These would need to be well documented of course, we could do something similar to the main part of the repo where the folder name is the proper name for the commits but the actual crate name is longer and more specific.

@airidaceae
Copy link
Collaborator

Also I think that this should be in the documentation milestone. Not kindling.

@marceline-cramer
Copy link
Collaborator Author

Also I think that this should be in the documentation milestone. Not kindling.

understood

@marceline-cramer
Copy link
Collaborator Author

marceline-cramer commented Dec 6, 2023

Another option is that we could use the subdirectory:
core/schema: ...
kindling/init: ...
tools/depgraph: ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants