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

Fix round-tripping commits that contain extra headers #1389

Merged

Conversation

bjeanes
Copy link
Contributor

@bjeanes bjeanes commented Sep 17, 2024

Fixes #1377
Closes #1387

As per #1387 (comment) this is how this might look by using the gix-object crate to own the commit object format.

- Fmt for Oid instead of hex::encode
- Into instead of as_bstr()
- ByteSlice instead of pointer deref and as_bstr()
Copy link
Member

@christian-schilling christian-schilling left a comment

Choose a reason for hiding this comment

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

Thanks for this! I think it's ok to use gix here.

josh-core/src/history.rs Outdated Show resolved Hide resolved
josh-core/src/history.rs Outdated Show resolved Hide resolved
tests/proxy/roundtrip_custom_header.t Outdated Show resolved Hide resolved
@bjeanes
Copy link
Contributor Author

bjeanes commented Sep 18, 2024

I think it's ok to use gix here.

Yeah once I realised (after doing #1387) that it was possible to use gix without using gix's odb or repo, then this was my strong preference for approach. I'm glad you agree. I'll close #1387.

What do you want to do about the /version endpoint and test? I changed the test but I'm not sure if the latest git tag on the repo was meant to be r24.08.14 instead of v24.08.14, to match the previous scheme.

@bjeanes bjeanes changed the title Fix round-tripping commits that contain extra headers (gix take) Fix round-tripping commits that contain extra headers Sep 18, 2024
Copy link
Member

@christian-schilling christian-schilling left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks a lot!

@christian-schilling christian-schilling merged commit 7dffff0 into josh-project:master Sep 23, 2024
1 check passed
@bjeanes
Copy link
Contributor Author

bjeanes commented Sep 23, 2024

Thank you @christian-schilling! What's your approach for release schedule. Now that this is fixed, a release including the fix will unblock my team's adoption of JOSH. Is this something that could be done?

@bjeanes bjeanes deleted the roundtrip-custom-headers-gix branch September 23, 2024 23:27
@christian-schilling
Copy link
Member

Thank you @christian-schilling! What's your approach for release schedule. Now that this is fixed, a release including the fix will unblock my team's adoption of JOSH. Is this something that could be done?

Out approach to release schedule is to release from time to time when major bugs of features where fixed or introduced. But we do run production instances straigt of master.
I did tag a new release.

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.

Roundtripping broken when raw commit includes custom metadata (e.g. as GitButler adds)
3 participants