radicle-dev / radicle-git

Everything Radicle growing around Git
Other
35 stars 5 forks source link

radicle-git-ext: merge in git-commit #118

Closed keepsimple1 closed 1 year ago

keepsimple1 commented 1 year ago

This is to address issue #114 .

author.rs commit.rs headers.rs

t/src/commit.rs

git-commit crate is not deleted because it's a published crate. Some users might depend on it.

This is because git-ref-format-macro uses proc-macro that AFAIK cannot mix into the same crate as radicle-git-ext. I don't have a good idea to work around this limit at this point. Maybe in future?

Note that radicle-macros and radicle-git-types are not published on crates.io, hence I think it's safe to remove them. Let me know otherwise.

FintanH commented 1 year ago

This is because git-ref-format-macro uses proc-macro that AFAIK cannot mix into the same crate as radicle-git-ext. I don't have a good idea to work around this limit at this point. Maybe in future?

Can you explain what the issues were?

keepsimple1 commented 1 year ago

This is because git-ref-format-macro uses proc-macro that AFAIK cannot mix into the same crate as radicle-git-ext. I don't have a good idea to work around this limit at this point. Maybe in future?

Can you explain what the issues were?

There are two issues I encountered: (I'm a newbie in proc-macros).

  1. proc-macros have to be defined in root source files in a crate. This is relatively easy to solve: we can put them under src/ in this crate.
  2. A crate cannot define both public proc-macros and public regular functions.

After researching a bit more, I found that we can also solve item 2 by treating git-ref-format-macro like an internal crate, and re-export all public macros in the parent crate. Such pattern is used in some common crates, like in thiserror.

keepsimple1 commented 1 year ago

I've tried out a patch for moving git-ref-format under radicle-git-ext, but it is quite involved. I think it's better to do that in a separate PR. I will post that patch in a new PR after this PR is landed.