yunikorn-k8shim icon indicating copy to clipboard operation
yunikorn-k8shim copied to clipboard

[YUNIKORN-2098] Change go lint SHA detection (following)

Open doupache opened this issue 1 year ago • 5 comments

What is this PR for?

Currently, we will always use the "ORIGIN/HEAD" ref.(In Github Action env) Fallback to "HEAD^" when "ORIGIN/HEAD" doesn't exist. Also print what git ref the ci have.

I get fatal: Needed a single revision on my fork github action ci. https://github.com/doupache/yunikorn-k8shim/actions/runs/6713607756/job/18245448950 image

image image

CI in the forked repo doesn't seem to have the "ORIGIN/HEAD" ref. I keep getting the 'fatal: Needed a single revision' error, similar to YUNIKORN-285

What type of PR is it?

  • [x] - Bug Fix

What is the Jira issue?

YUNIKORN-2098

doupache avatar Nov 01 '23 00:11 doupache

@wilfred-s Please take a look !

This is the following PR of https://github.com/apache/yunikorn-k8shim/pull/210/

doupache avatar Nov 01 '23 00:11 doupache

I have the same problem in my fork project. link: https://github.com/targetoee/yunikorn-k8shim/actions/runs/6716879779/job/18253781242

targetoee avatar Nov 01 '23 07:11 targetoee

Pass on my fork with branch doupache:YUNIKORN-2098

https://github.com/doupache/yunikorn-k8shim/actions/runs/6714213847

It seems there is another issue with the Pull Request CI environment I need Committer to approve workflow , help me fix the bug.

doupache avatar Nov 01 '23 13:11 doupache

In YUNIKORN-285, we previously included git symbolic-ref -q HEAD because Travis CI often encountered a detached head situation. However, now that we have switched to GitHub Actions, we can utilize the checkout@v3 action, which efficiently fetches all Git references.

Consequently, we can remove git symbolic-ref -q HEAD Instead, we can initially set REV='origin/HEAD' If git rev-parse fails, we can fall back to REV=HEAD^

Regarding the modification from fetch-depth: 2 to fetch-depth: 0, I have compared the GitHub Action times before this pull request and with the master. The duration of the git checkout action remained around 2 seconds. Therefore, I believe this change is acceptable, as it does not introduce significant overhead.

@wilfred-s I think this PR is ready for review.

doupache avatar Nov 27 '23 07:11 doupache

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (779450c) 69.52% compared to head (13cfd67) 69.46%. Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #715      +/-   ##
==========================================
- Coverage   69.52%   69.46%   -0.07%     
==========================================
  Files          50       50              
  Lines        7993     7993              
==========================================
- Hits         5557     5552       -5     
- Misses       2248     2252       +4     
- Partials      188      189       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 27 '23 23:11 codecov[bot]

With the cleanup for lint issues we no longer need SHA detection in our lint runs. It was removed as part of an earlier change.

wilfred-s avatar Aug 14 '24 06:08 wilfred-s