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

git-cliff-core: allow adding arbitrary values to commits #803

Closed
1 task done
MarcoIeni opened this issue Aug 19, 2024 · 6 comments · Fixed by #822
Closed
1 task done

git-cliff-core: allow adding arbitrary values to commits #803

MarcoIeni opened this issue Aug 19, 2024 · 6 comments · Fixed by #822
Assignees
Labels
feature/request New feature or request

Comments

@MarcoIeni
Copy link
Contributor

MarcoIeni commented Aug 19, 2024

Is there an existing issue or pull request for this?

  • I have searched the existing issues and pull requests

Feature description

First of all thank you for the amazing crate and the hard work you put into it 🙏

I would like to attach a remote context to the commit, so that people can write commit.remote.username in release-plz changelog (which is similar to commit.github.username git-cliff equivalent.

Even though I can't have commit.remote I tried enabling the "github" feature and my Cargo.lock exploded because of many dependencies to retrieve remote info I'm not using (because I'm retrieving remote info by myself). So I would like to have a "lighter" way to add arbitrary fields to the commit object.

So a very similar mechanism to #613 but limited to the commit object.

Desired solution

The Commit struct could have a Map of <String, Json> (this is psudo-code). In my example, "String" is remote and Json is { author }.

Library users should be able to add any fields they like to it.

Alternatives considered

The alternative is rely entirely on git-cliff for commits remote info retrieval.

My issue is that I generate multiple changelogs that share some commits, so we would hit GitHub API multiple times for the same commits for no reason.
We could solve this by implementing #468 but I'm not sure if it's easier than this issue.

Also, with this approach, the following problems remain:

  • I would like to have commit.remote instead of commit.github, commit.gitea, etc. (unless you change my mind that commit.github is a better idea 👀)
  • I prefer to avoid adding all those dependencies because release-plz already takes a while to compile 😵

Additional context

PR where I experimented: release-plz/release-plz#1630

Also, I'm curious, why you have commit.github, commit.gitea, etc instead of a unified solution (e.g. commit.remote)?

@MarcoIeni
Copy link
Contributor Author

I just thought about the simplest solution to my issue: the Commit struct adds a remote field as Option. This field is not behind a feature flag (so I don't need to pull other dependencies).

Of course it's not a generic solution 👍

@orhun
Copy link
Owner

orhun commented Aug 20, 2024

Hello Marco 👋🏼

The Commit struct could have a Map of <String, Json> (this is psudo-code). In my example, "String" is remote and Json is { author }.

Would that work? Is serde serializing keys as separate fields?

Btw can you take a look at #784 which does something similar (adds a new field called extra). Maybe we can tweak PR a bit to satisfy this use case too.

Also, I'm curious, why you have commit.github, commit.gitea, etc instead of a unified solution (e.g. commit.remote)?

There is a stale PR #688 which greatly simplifies that module.

@MarcoIeni
Copy link
Contributor Author

MarcoIeni commented Aug 24, 2024

Would that work? Is serde serializing keys as separate fields?

I guess we would need to do what the linked PR is doing:

image

(adds a new field called extra). Maybe we can tweak PR a bit to satisfy this use case too.

Yeah, I would need something like that, but without typing extra. So as the remote HashMap 👍

Adding another field called extra_map which is an HashMap would work for example.

@orhun
Copy link
Owner

orhun commented Aug 26, 2024

This feels like a bit of a workaround and is specific to release-plz's use case.

Maybe we should deprecate individual remote-related fields and instead support commit.remote, as you've suggested. That way, we wouldn't need to make small tweaks in the library like this.

@MarcoIeni
Copy link
Contributor Author

MarcoIeni commented Aug 27, 2024

Sounds good to me as long as the remote field is not behind a feature flag that pulls other dependencies 👍

Should I create a pr that adds this field?
Maybe for now we can just add the field without filling it so that it unlocks release-plz. I.e. it's a field reserved to library users that git cliff can use later after a refactoring

@orhun
Copy link
Owner

orhun commented Aug 27, 2024

Yup, please go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/request New feature or request
Projects
None yet
2 participants