Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

*: switch to dep for dependency management #876

Merged
merged 2 commits into from
Feb 6, 2018
Merged

*: switch to dep for dependency management #876

merged 2 commits into from
Feb 6, 2018

Conversation

ericchiang
Copy link
Contributor

No description provided.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 5, 2018
@@ -0,0 +1,15 @@
#!/bin/bash -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you notice anything that fails this check so far? Or is this an assert style sanity check?

This also doesn't seem to assert that the assets in the vendor/ are valid corresponding to either the toml/lock. Is that worth doing as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assert style sanity check.


# NOTE(ericchiang): For some reason you need this "export" here for bash reasons
# I don't understand.
export DIFF=$( diff <(echo "$TOML") <(echo "$LOCK") )
Copy link
Contributor

Choose a reason for hiding this comment

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

Given you comment, and a quick test, did you ensure this works in the failure case?

You don't wrap the value in quotes, and DIFF= and export DIFF= work differently if the values have spaces, at least in my quick little experiment it seemed to.

It seems like maybe export is papering over the fact that your value has whitespace and allowing for false positives?

Copy link
Contributor

Choose a reason for hiding this comment

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

might need a -q in there as well to only print when files differ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I confirmed that this works when the files are out of sync.

If I comment out the export and modify one of the Gopkg files I can replicate locally that DIFF becomes set to the empty string no matter what.

I can add some quotes, though I tested that and it didn't work to fix this problem. @thorfour and I were staring at this for a while and don't have any good answer here.


# NOTE(ericchiang): For some reason you need this "export" here for bash reasons
# I don't understand.
export DIFF=$( diff <(echo "$TOML") <(echo "$LOCK") )
Copy link
Contributor

Choose a reason for hiding this comment

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

should be:

DIFF=$( diff <(echo "$TOML") <(echo "$LOCK") || true )

diff will return an error code if there is a difference causing the script to exit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it worked!

@ericchiang
Copy link
Contributor Author

Gonna wait for #877 to merge to push the change.

@rphillips
Copy link
Contributor

merged #877

@ericchiang ericchiang merged commit b356bdd into kubernetes-retired:master Feb 6, 2018
@ericchiang ericchiang deleted the dep branch February 6, 2018 05:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants