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

build: use git default short commit-id #7761

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Conversation

JayFate
Copy link
Contributor

@JayFate JayFate commented Feb 20, 2023

before this commit

const commit = execa.sync('git', ['rev-parse', 'HEAD']).stdout.slice(0, 7)
console.log(`commit`, commit) 
// commit a0e7dc3

after this commit

const commit = execa.sync('git', ['rev-parse', '--short', 'HEAD']).stdout
console.log(`commit`, commit) 
// commit be78cf43

@JayFate
Copy link
Contributor Author

JayFate commented Feb 20, 2023

@sxzz Do you have a minute to review this PR ? THX

@yyx990803
Copy link
Member

What is the point of this? The original uses 7 chars which is consistent with the GitHub short format.

@JayFate JayFate closed this Feb 22, 2023
@sxzz
Copy link
Member

sxzz commented Feb 22, 2023

@yyx990803

It may create a non-unique commit id by slicing 7 lengths. (Although the current probability is very unlikely)

you can ask git rev-parse --short for the shortest and yet unique SHA1

Further read: https://stackoverflow.com/questions/18134627/how-much-of-a-git-sha-is-generally-considered-necessary-to-uniquely-identify-a

@sxzz sxzz reopened this Feb 22, 2023
@Justineo
Copy link
Member

After looking at where we are using these short commit ids, I think we should keep this consistent with GitHub’s format.

@JayFate
Copy link
Contributor Author

JayFate commented Apr 3, 2023

@yyx990803 @sxzz @Justineo Updated.

Is this commit necessary any more ?

@github-actions
Copy link

github-actions bot commented Aug 21, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 85.9 kB 32.7 kB 29.5 kB
vue.global.prod.js 132 kB 49.3 kB 44.3 kB

Usages

Name Size Gzip Brotli
createApp 47.9 kB 18.8 kB 17.2 kB
createSSRApp 50.6 kB 20 kB 18.2 kB
defineCustomElement 50.3 kB 19.6 kB 17.9 kB
overall 61.2 kB 23.7 kB 21.6 kB

Signed-off-by: JayFate <48240828+JayFate@users.noreply.github.com>
@yyx990803 yyx990803 merged commit 21a89af into vuejs:main Oct 20, 2023
11 checks passed
lumozx pushed a commit to lumozx/core that referenced this pull request Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy for merge ready to merge The PR is ready to be merged.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants