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

Add tag #15

Closed
wants to merge 1 commit into from
Closed

Add tag #15

wants to merge 1 commit into from

Conversation

minhluong96
Copy link

Add git describe --tag into result

@nexdrew
Copy link
Owner

nexdrew commented Apr 3, 2019

@minhluong96 Thanks for the contribution! 😺 I like this idea, but I have a couple concerns:

  1. Does it make sense to always provide results of both git rev-parse HEAD and git describe --tags, or should the consumer specify which one they want (and then it just gets returned as the id result)? I can imagine an option like a boolean useTag or maybe even a string (or array of strings) that specifies the git command to use.

  2. If we do always just return both, then it seems like (a) we could possibly run both git commands concurrently via Promise.all() and (b) we could easily refactor the code to reduce duplicate logic between the determineBuildTag and determineBuildId functions.

  3. We need to fix broken tests, add new assertions for the result of the tag prop, and add a new test for when tag is predefined in opts.

Let me know what you think.

@minhluong96 minhluong96 closed this Apr 4, 2019
@minhluong96 minhluong96 reopened this Apr 4, 2019
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.

None yet

2 participants