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: change Commit#String format #404

Merged
merged 4 commits into from
Feb 10, 2023
Merged

git: change Commit#String format #404

merged 4 commits into from
Feb 10, 2023

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Nov 14, 2022

This changes the string format of Commit to use an @ separator instead of /, and drops the usage of "HEAD" as a virtual named reference for commits without a named pointer (e.g. a Git branch and/or tag). Matching the (revision) format proposed in RFC-0005.

The LastObservedCommit option is made backwards compatible, allowing consumers to provide a revision string in the previous format without issues.

To help consumers address possible compatibility issues while working with the new (returned) format, the following functions have been added:

  • TransformRevision: allows for transforming a "legacy revision" (i.e. feature/branch/5394cb7f48332b2de7c17dd8b8384bbc84b7e738 or HEAD/5394cb7f48332b2de7c17dd8b8384bbc84b7e738) into a RFC-0005 compatible revision (i.e. feature/branch@sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738 or sha1:5394cb7f48332b2de7c17dd8b8384bbc84b7e738).
  • SplitRevision: allows for a legacy or RFC-0005 revision string to be split into a named-pointer and/or hash.
  • ExtractNamedPointerFromRevision: allows for extracting a named pointer from a legacy or RFC-0005 revision string.
  • ExtractHashFromRevision: allows for extracting a hash from a legacy or RFC-0005 revision string.

@hiddeco hiddeco force-pushed the commit-string-fmt branch 8 times, most recently from f8fd945 to 721d447 Compare November 16, 2022 10:05
@hiddeco hiddeco force-pushed the commit-string-fmt branch 3 times, most recently from f62b6da to 0360540 Compare December 13, 2022 23:09
@hiddeco hiddeco added area/git Git and SSH related issues and pull requests enhancement New feature or request labels Dec 13, 2022
@hiddeco hiddeco requested a review from pjbgf December 13, 2022 23:10
@hiddeco hiddeco marked this pull request as ready for review December 13, 2022 23:22
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

LGTM

Great job @hiddeco! 🙇

@hiddeco hiddeco added the hold Issues and pull requests put on hold label Dec 14, 2022
@hiddeco
Copy link
Member Author

hiddeco commented Dec 14, 2022

Given we are also optimizing some other bits of the Git package at present to facilitate the removal of libgit2 and address other issues. I suggest we keep this on hold until I have my proposed changes incorporating RFC-0005 for source-controller ready.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

This changes the string format of `Commit` to use an `@` separator
instead of `/`, and drops the usage of "HEAD" as a virtual named
reference for commits without a named pointer (e.g. a Git branch and/or
tag). Matching the (revision) format proposed in RFC-0005.

Signed-off-by: Hidde Beydals <[email protected]>
Signed-off-by: Hidde Beydals <[email protected]>
This ensures the Git implementations transform any LastObservedCommit
value into the new format before comparing it to what they constructed
from the remote state.

Signed-off-by: Hidde Beydals <[email protected]>
@hiddeco hiddeco removed the hold Issues and pull requests put on hold label Feb 10, 2023
@hiddeco hiddeco merged commit 0009fda into main Feb 10, 2023
@hiddeco hiddeco deleted the commit-string-fmt branch February 10, 2023 12:52
hiddeco added a commit that referenced this pull request Feb 10, 2023
This ensures this strategy is backwards compatible as well, as I
accidentally forgot to do this for go-git in #404.

This lacks tests (as does the libgit2 implementation), and it would
probably be good to make them less dynamic as a follow up, as that's
how this slipped through the cracks.

Signed-off-by: Hidde Beydals <[email protected]>
hiddeco added a commit that referenced this pull request Feb 10, 2023
This ensures this strategy is backwards compatible as well, as I
accidentally forgot to do this for go-git in #404.

This lacks tests (as does the libgit2 implementation), and it would
probably be good to make them less dynamic as a follow up, as that's
how this slipped through the cracks.

Signed-off-by: Hidde Beydals <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git and SSH related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants