Skip to content

Allow overriding repository and branch in validate scripts#38655

Merged
tianon merged 1 commit intomoby:masterfrom
thaJeztah:override_validate
Feb 1, 2019
Merged

Allow overriding repository and branch in validate scripts#38655
tianon merged 1 commit intomoby:masterfrom
thaJeztah:override_validate

Conversation

@thaJeztah
Copy link
Member

When running CI in other repositories (e.g. Docker's downstream
docker/engine repository), or other branches, the validation
scripts were calculating the list of changes based on the wrong
information.

This lead to weird failures in CI in a branch where these values
were not updated ':-) (CI on a pull request failed because it detected
that new tests were added to the deprecated integration-cli test-suite,
but the pull request did not actually make changes in that area).

This patch allows overriding the target repository (and branch)
to compare to (without having to edit the scripts).

@thaJeztah
Copy link
Member Author

ping @tianon PTAL

I was looking for a way to programatically figure out what the "target" branch (VALIDATE_BRANCH) was, but my Git-fu failed me there. I realize you could find it with GitHub's API, but is there a way to figure out using, e.g. some git rev-parse or git branch --points-at magic?

If so, that would ease configuration (i.e., no need to configure these environment variables)

@tianon
Copy link
Member

tianon commented Jan 30, 2019

I was looking for a way to programatically figure out what the "target" branch (VALIDATE_BRANCH) was, but my Git-fu failed me there. I realize you could find it with GitHub's API, but is there a way to figure out using, e.g. some git rev-parse or git branch --points-at magic?

Just to proxy some of my thoughts back from our Slack conversation for posterity:

nope, probably not -- which is why we had to do that fetching nonsense in the first place'

GitHub's API is annoying because rate limited

but otherwise it's realllllly annoying to try and get a proper diff of changes against the "target branch" because you can't really know from just Git what the target branch is

the best you can do is guess from common commits, but even that's going to be a bit rubbish

When running CI in other repositories (e.g. Docker's downstream
docker/engine repository), or other branches, the validation
scripts were calculating the list of changes based on the wrong
information.

This lead to weird failures in CI in a branch where these values
were not updated ':-) (CI on a pull request failed because it detected
that new tests were added to the deprecated `integration-cli` test-suite,
but the pull request did not actually make changes in that area).

This patch allows overriding the target repository (and branch)
to compare to (without having to edit the scripts).

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@44af96c). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #38655   +/-   ##
=========================================
  Coverage          ?   36.58%           
=========================================
  Files             ?      610           
  Lines             ?    45217           
  Branches          ?        0           
=========================================
  Hits              ?    16541           
  Misses            ?    26396           
  Partials          ?     2280

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member Author

@tianon LGTY? 🤗

@tianon
Copy link
Member

tianon commented Feb 1, 2019

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants