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

Revert "Use action/checkout fetch-depth instead of manually fetch (#4535)" #4560

Merged
merged 1 commit into from
Dec 16, 2021

Conversation

jpkrohling
Copy link
Member

@jpkrohling jpkrohling commented Dec 16, 2021

This reverts commit 5405247.

The problem is happening because there's a mismatch between references: previously, the changelog was obtaining the latest commit from github.base_ref, which is typically main. The referenced commit uses the GitHub Action, which will get the latest commit from the PR itself. Running a diff between the PR against itself yields no results at all:

$ git fetch origin jpkrohling/issue4556 --depth=1
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
From github.com:jpkrohling/opentelemetry-collector
 * branch              jpkrohling/issue4556 -> FETCH_HEAD

$ git diff --name-only FETCH_HEAD
$

Whereas running the same command using main in the FETCH_HEAD yields

$ git fetch origin main --depth=1
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
From github.com:jpkrohling/opentelemetry-collector
 * branch              main       -> FETCH_HEAD

$ git diff --name-only FETCH_HEAD
CHANGELOG.md
config/configauth/configauth_test.go
config/configauth/default_serverauthenticator.go
config/configauth/default_serverauthenticator_test.go
config/configauth/mock_serverauth.go
config/configauth/mock_serverauth_test.go
config/configgrpc/configgrpc_test.go
config/confighttp/confighttp_test.go

Fixes #4559

@jpkrohling jpkrohling requested review from a team and dmitryax December 16, 2021 11:08
@codecov
Copy link

codecov bot commented Dec 16, 2021

Codecov Report

Merging #4560 (cfedf11) into main (5514ecb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4560   +/-   ##
=======================================
  Coverage   90.50%   90.50%           
=======================================
  Files         180      180           
  Lines       10545    10545           
=======================================
  Hits         9544     9544           
  Misses        782      782           
  Partials      219      219           

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 5514ecb...cfedf11. Read the comment docs.

# Only the latest commit of the feature branch is available
# automatically. To diff with the base branch, we need to
# fetch that too (and we only need its latest commit).
fetch-depth: 1
Copy link
Member Author

Choose a reason for hiding this comment

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

According to the documentation, fetch-depth: 1 is already the default, so, this is a noop. https://github.com/actions/checkout#usage

@jpkrohling jpkrohling added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 16, 2021
@codeboten codeboten added the ready-to-merge Code review completed; ready to merge by maintainers label Dec 16, 2021
@bogdandrutu bogdandrutu merged commit cf5aa32 into open-telemetry:main Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Code review completed; ready to merge by maintainers Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changelog check is failing
4 participants