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

fix: use 8 characters for commit SHAs #372

Merged
merged 1 commit into from Oct 31, 2019
Merged

fix: use 8 characters for commit SHAs #372

merged 1 commit into from Oct 31, 2019

Conversation

targos
Copy link
Member

@targos targos commented Oct 24, 2019

GitHub already refused to generate a link with 7.

@targos
Copy link
Member Author

targos commented Oct 24, 2019

See nodejs/node#30064 (comment)

This link works: nodejs/node@d53dd8b0
This link doesn't: nodejs/node@d53dd8b

@addaleax
Copy link
Member

Fwiw, I’ve set this to 12 locally, and I’m happy with that. But since most messages here either aren’t user-facing or are supposed to end up on Github where they will be automatically abbreviated anyway, can we maybe go for something even longer or the full SHA?

@targos
Copy link
Member Author

targos commented Oct 24, 2019

I'm fine with any length

GitHub already refused to generate a link with 7.
@targos
Copy link
Member Author

targos commented Oct 24, 2019

Increased to 12, created an utility function and fixed tests.

@codecov
Copy link

codecov bot commented Oct 24, 2019

Codecov Report

Merging #372 into master will increase coverage by 0.05%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
+ Coverage   75.23%   75.28%   +0.05%     
==========================================
  Files          21       21              
  Lines        1409     1412       +3     
==========================================
+ Hits         1060     1063       +3     
  Misses        349      349
Impacted Files Coverage Δ
lib/ci/ci_result_parser.js 46.06% <0%> (ø) ⬆️
lib/utils.js 100% <100%> (ø) ⬆️
lib/pr_checker.js 96.35% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd7318d...7350877. Read the comment docs.

@targos targos merged commit a9c0676 into nodejs:master Oct 31, 2019
@targos targos deleted the sha-8 branch October 31, 2019 09:58
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