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

refactoring: call git via execSync only once #5

Merged

Conversation

Snaptags
Copy link
Contributor

@Snaptags Snaptags commented May 2, 2020

You can use a custom git log format to extract all required information at once. The available placeholders (https://git-scm.com/docs/git-log#_pretty_formats) are quite powerful, you can even use them to generate a JSON string to be parsed automatically.

For this I added two functions:

  • gitLogToJSON: defines the custom format and runs git once to get the result. There are two problems to consider here: at least git commit messages may contain quotes (") as well as new line characters, both requiring special treatment in a JSON string. To handle both cases I use a very weird separator string to be able to first escape all quotes inside the values. After that the placeholder is replaced by quote to get a valid JSON. This of course will break if anyone actually uses the placeholder string in a commit message. Not very likely to happen :-)
  • parseRefs: git's %D format placeholder returns refs as comma-separated string, e.g. "HEAD -> master, tag: v1.1.0, tag: bugfix". The function parses this into the branch name and an array of tag names.

This might not be the very best solution, but at least we don't have to call git 6 times.

@AbhyudayaSharma AbhyudayaSharma self-requested a review May 2, 2020 12:14
Copy link
Owner

@AbhyudayaSharma AbhyudayaSharma 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 your contributions @Snaptags ! I always wanted to do this.

This looks fine but there are some things that may cause issues. I think we can probably do all this with three execSync calls.

  1. Commit details using git log '--format=%h%n%H%n%cI%n%B'
  2. Tags using git tag --list --points-at HEAD
  3. Current branch using git rev-parse --abbrev-ref HEAD

What do you think about this?

I wish we could've gotten the current branch using git log but pretty-formats has nothing about getting branch names.

src/GitInfo.macro.js Outdated Show resolved Hide resolved
src/GitInfo.macro.js Outdated Show resolved Hide resolved
@AbhyudayaSharma
Copy link
Owner

I think it is better to wait for this to get merged too before cutting a new release consisting of just #4. If you need #4 urgently, let me know.

@AbhyudayaSharma AbhyudayaSharma self-requested a review May 4, 2020 04:26
Copy link
Owner

@AbhyudayaSharma AbhyudayaSharma left a comment

Choose a reason for hiding this comment

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

Thanks again @Snaptags ! Looks perfect now, just some minor suggestions.

src/GitInfo.macro.js Outdated Show resolved Hide resolved
src/GitInfo.macro.js Outdated Show resolved Hide resolved
src/GitInfo.macro.js Outdated Show resolved Hide resolved
src/GitInfo.macro.js Outdated Show resolved Hide resolved
@AbhyudayaSharma AbhyudayaSharma merged commit 006e043 into AbhyudayaSharma:master May 4, 2020
@AbhyudayaSharma
Copy link
Owner

Thanks @Snaptags !

@AbhyudayaSharma AbhyudayaSharma added the enhancement New feature or request label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants