Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Consolidate ProjectIndentifier sorts; drop build support for go1.7 #714

Merged
merged 2 commits into from
Aug 29, 2017
Merged

Consolidate ProjectIndentifier sorts; drop build support for go1.7 #714

merged 2 commits into from
Aug 29, 2017

Conversation

jmank88
Copy link
Collaborator

@jmank88 jmank88 commented Jun 3, 2017

#672
This change proposes consolidating ProjectIdentifier based sorting code in order to reduce exported API surface, and DRY up internal code.

There were three slightly different methods of ordering ProjectIdentifier:

  1. ProjectRoot only
  2. ProjectRoot, then Source
  3. ProjectRoot, then normalizedSource()

Now the sole Less method behind every sort is the third version.

There is a dependency on sort methods introduced in go 1.8, so CI is expected to partially fail for now.

@ibrasho
Copy link
Collaborator

ibrasho commented Jun 15, 2017

Related: #757

@sdboyer
Copy link
Member

sdboyer commented Jul 26, 2017

we still feel this is relevant, right?

i think, once Go 1.9 is out, i'm fine with abandoning 1.7, so we can actually use sort.Slice().

@jmank88
Copy link
Collaborator Author

jmank88 commented Jul 26, 2017

I'll have to rebase but it should still be partially relevant

@sdboyer
Copy link
Member

sdboyer commented Aug 18, 2017

we'll hold off on merging this and dropping the 1.7 support until there's a full release of 1.9.

in the meantime, this'll need to drop the 1.7 builder in travis - let's replace that one with 1.9. also, update the README.

@jmank88
Copy link
Collaborator Author

jmank88 commented Aug 18, 2017

Ok to yank things tagged to build for <=1.7 too?
Edit: I did it.

@jmank88 jmank88 changed the title Consolidate ProjectIndentifier sorts Consolidate ProjectIndentifier sorts; drop build support for go1.7 Aug 18, 2017
@sdboyer
Copy link
Member

sdboyer commented Aug 19, 2017 via email

@jmank88 jmank88 closed this Aug 28, 2017
@jmank88 jmank88 reopened this Aug 28, 2017
@@ -27,7 +27,7 @@ jobs:
- codeclimate-test-reporter < coverage.txt
# YAML alias, for settings shared across the simpler builds
- &simple-test
go: 1.7.x
go: 1.9.x
Copy link
Member

Choose a reason for hiding this comment

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

i think it needs to be just 1.9

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was holding out for them to update and it looks like they did at some point today. Travis is now recognizing 1.9.x and passing only 1.9 to gimme. Strange that they don't have a general rule for picking up 1.Y.X for any Y though.

@jmank88 jmank88 closed this Aug 29, 2017
@jmank88 jmank88 reopened this Aug 29, 2017
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

weee!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants