search

jacob-meacham / serverless-plugin-git-variables

:zap: Expose git variables to serverless
MIT License
87 stars 32 forks source link

Better value for git:branch when in detached state #63

Closed JordanReiter closed 3 years ago

JordanReiter commented 3 years ago

Certain CI services may deploy a git repo by checking out a specific commit rather than the branch. When this happens, the repo is in a "detached" state. Currently, this causes the value for git:branch to always be HEAD instead of the name of the branch associated with the commit.

If the commit happens to be the last commit of the branch, it is possible to retrieve the branch name and use that instead of HEAD.

This commit makes a minor change to the branch case in _getValueFromGit so that if the initial value is HEAD it attempts to determine the correct branch name, falling back on HEAD if it is not able to.

This commit includes 2 test cases to ensure correct behavior along with a test repo multiple_commits used for testing.

JordanReiter commented 3 years ago

If you're willing, I would appreciate it if you could mark this PR with the label hacktoberfest-accepted so it can count towards this year's Hacktoberfest. If not, I still hope you find this PR helpful. It would certainly be useful for our team, which experiences the problem described (git:branch displaying as "HEAD" because our CI tools deploy from a detached head state).

coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.04%) to 98.837% when pulling 5b4ddaac606d1e812279272b2868b56e4d5f6efa on JordanReiter:detached-head-branch into c7658e3f12595d54b77fad5a74be8200884fcf6d on jacob-meacham:develop.

james-hu commented 3 years ago

I am just passing by. I like this functionality, it addresses a real need. However this is also a breaking change, I would suggest to add a configuration option for toggling this behaviour on or off.

JordanReiter commented 3 years ago

I am just passing by. I like this functionality, it addresses a real need. However this is also a breaking change, I would suggest to add a configuration option for toggling this behaviour on or off.

Just to clarify: you want it to be possible for users to indicate that they want the value for git:branch to report HEAD when in a detached state?

Or is there another way that this is a breaking change?

JordanReiter commented 3 years ago

@JordanReiter thanks for the PR, this is great! I agree that adding a flag here would be good (which we can then remove at the next major release)

Sorry for the late follow-up. Let me know how you'd like me to implement the flag and I'll add it into the code, along with a warning.

jacob-meacham commented 3 years ago

Maybe a predictDetatchedBranch value in the config, which can default to false for now

jacob-meacham commented 3 years ago

Would love to merge this in if you'd like to make the requested changes @JordanReiter. Closing for now