Skip to content

gix-actor docs: document conversions between Signature and SignatureRef #2038

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 6, 2025
Merged

gix-actor docs: document conversions between Signature and SignatureRef #2038

merged 2 commits into from
Jun 6, 2025

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Jun 5, 2025 •
edited
Loading

These are some docs that could have been helpful as I am adapting jj to #1935. Please feel free to edit them further.

I guess a part of this PR is a question: did I figure out correctly how I'm supposed to use these?

See also #1935 (comment). I wonder if some of these classes and methods should be renamed. (Perhaps it's possible to make the old name a deprecated type alias?)

@ilyagr
Copy link
Contributor Author

ilyagr commented Jun 5, 2025 •
edited
Loading

I also noticed that there exists git_mailmap::Signature, but I'm guessing I should not be using that.

Then there's impl From<SignatureRef<'_>> for Signature {, which I perhaps should use instead of SignatureRef::to_owned() (perhaps that should be called parse?). I thought this was a TryFrom (Update: this is what the changelog for gix-actor said), but I guess that changed (or my IDE failed to find the TryFrom).

I'm also not sure whether the fact that the From implementation is lossy should be documented or not.

@ilyagr ilyagr mentioned this pull request Jun 5, 2025
Merged
4 tasks
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
@ilyagr
post-update signature stuff
6fe3e58 
Adapt to changes in `gix-actor::Signature` and `gix-actor::SignatureRef`. The latter now contains an unparsed
string time field, while the former still contains a parsed time.
So, the conversions between `gix-actor::SignatureRef` and either `gix-actor::Signature` or jj's `Signature` types can now fail.

Cc: GitoxideLabs/gitoxide#1935, GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
@ilyagr
post-update signature stuff
37d5f09 
Adapt to changes in `gix-actor::Signature` and
`gix-actor::SignatureRef`. The latter now contains an unparsed string
time field, while the former still contains a parsed time.  So, the
conversions between `gix-actor::SignatureRef` and either
`gix-actor::Signature` or jj's `Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
cb611c9 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
14cfcf6 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
@ilyagr ilyagr mentioned this pull request Jun 6, 2025
Merged
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
ce4b48a 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
957aaad 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
bec00b9 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
Byron
Byron approved these changes Jun 6, 2025
View reviewed changes
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for improving the documentation!

I did edit it as suggested, mainly to be more specific where possible.

Then there's impl From<SignatureRef<'_>> for Signature {, which I perhaps should use instead of SignatureRef::to_owned() (perhaps that should be called parse?). I thought this was a TryFrom (Update: this is what the changelog for gix-actor said), but I guess that changed (or my IDE failed to find the TryFrom).

For consistency, one could add a TryFrom that calls to_owned() under the hood.
I prefer to_owned() if given the choice as I don't find try_from() very discoverable, but that might also be affected by the IDE.

Please feel free to submit a PR for that though.

@Byron Byron enabled auto-merge June 6, 2025 02:59
@Byron Byron merged commit 8f6ecfe into GitoxideLabs:main Jun 6, 2025
23 checks passed
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
9190952 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
github-merge-queue bot pushed a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
@ilyagr
git_backend: update gix, adapt to breaking changes
ff63a7e 
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
@ilyagr ilyagr deleted the signature-doc branch June 8, 2025 03:41
@ilyagr
Copy link
Contributor Author

ilyagr commented Jun 8, 2025

Thank you very much for the review! Your edits are also great; the final result seems clear and helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Byron Byron Byron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ilyagr @Byron