Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.

Conversation

abbytiz
Copy link
Contributor

@abbytiz abbytiz commented Aug 10, 2017

@nkubala @cftorres @aaron-prindle
#2

(Ideally to be merged before #1 so that pip differ is working accurately before being expanded to more package locations)

.gofmt.sh Outdated

set -e

files=$(find . -name "*.go" | grep -v iDiff/vendor | grep -v vendor/ | xargs gofmt -l -s)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the grep -v iDiff/vendor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

.gofmt.sh Outdated
exit 1
fi

files=$(go vet ./structure_tests)
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@abbytiz abbytiz force-pushed the multiVersionGeneralization branch from 2c429bf to b594383 Compare August 10, 2017 18:38
return 0, false
}

func multiVersionDiff(infoDiff []MultiVersionInfo, key string, map1, map2 map[string]PackageInfo) []MultiVersionInfo {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe more descriptive vars below besides "key" and "value" types would be helpful to understanding what's going on here

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 reworked the key/value naming in this function and diffMaps. Hopefully it's a little clearer what is happening.

@cftorres
Copy link
Contributor

lgtm

Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

LGTM, let's wait til travis is fixed before merging though

…s keys in the map to package info structs, adjusting multiversion differ to be less node specific, still need to adjust unit and integration tests accordingly.
…pyVersion folder didn't have sitepackages rather than looking through all of them
@abbytiz abbytiz force-pushed the multiVersionGeneralization branch from 10545f4 to e89d004 Compare August 10, 2017 22:45
@abbytiz abbytiz force-pushed the multiVersionGeneralization branch from e89d004 to 10aa5d3 Compare August 10, 2017 22:49
@abbytiz abbytiz merged commit 3a83f11 into GoogleContainerTools:master Aug 10, 2017
@abbytiz abbytiz deleted the multiVersionGeneralization branch August 10, 2017 22:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants