Skip to content

Add an Append.Sequence instance for List#2114

Merged
jsuereth merged 1 commit intosbt:0.13from
pdalpra:add-append.sequence-list
Jul 18, 2015
Merged

Add an Append.Sequence instance for List#2114
jsuereth merged 1 commit intosbt:0.13from
pdalpra:add-append.sequence-list

Conversation

@pdalpra
Copy link
Member

@pdalpra pdalpra commented Jul 17, 2015

Fixes #2107

@eed3si9n eed3si9n added the ready label Jul 17, 2015
@pdalpra
Copy link
Member Author

pdalpra commented Jul 17, 2015

Is there a place where I could add tests for this ?

@typesafe-tools
Copy link

typesafe-tools commented Jul 17, 2015

Can one of the admins verify this patch?

@eed3si9n
Copy link
Member

eed3si9n commented Jul 18, 2015

https://github.com/sbt/sbt/blob/0.13/main/settings/src/test/scala/UsageTest.scala looks like a place.

@jsuereth
Copy link
Member

jsuereth commented Jul 18, 2015

With tests, this would LGTM

@eed3si9n
Copy link
Member

eed3si9n commented Jul 18, 2015

And notes.

@pdalpra
Copy link
Member Author

pdalpra commented Jul 18, 2015

Amended my commit to add notes and tests.

dwijnand
dwijnand reviewed Jul 18, 2015
View reviewed changes
main/settings/src/test/scala/UsageTest.scala Outdated
Copy link
Member

@dwijnand dwijnand Jul 18, 2015

Choose a reason for hiding this comment

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

I think these 3 should be listSetting.

Copy link
Member Author

@pdalpra pdalpra Jul 18, 2015

Choose a reason for hiding this comment

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

Woops.

Copy link
Member Author

@pdalpra pdalpra Jul 18, 2015

Choose a reason for hiding this comment

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

fixed

@dwijnand
Copy link
Member

dwijnand commented Jul 18, 2015

LGTM, unless we want to also test that the view bound variant is also working.

@jsuereth
Copy link
Member

jsuereth commented Jul 18, 2015

LGTM

jsuereth added a commit that referenced this pull request Jul 18, 2015
@jsuereth
Merge pull request #2114 from pdalpra/add-append.sequence-list
d0e918d 
Add an Append.Sequence instance for List
@jsuereth jsuereth merged commit d0e918d into sbt:0.13 Jul 18, 2015
@jsuereth jsuereth removed the ready label Jul 18, 2015
@pdalpra pdalpra deleted the add-append.sequence-list branch July 18, 2015 23:12
marconilanna added a commit to marconilanna/sbt that referenced this pull request Aug 17, 2017
@marconilanna
Partially revert sbt#2114
26df39d
marconilanna added a commit to marconilanna/sbt that referenced this pull request Aug 17, 2017
@marconilanna
Partially revert sbt#2114
3b1f7e8
@marconilanna marconilanna mentioned this pull request Aug 17, 2017
Closed
marconilanna added a commit to marconilanna/sbt that referenced this pull request Aug 17, 2017
@marconilanna
Revert "Partially revert sbt#2114"
e184f7c 
This reverts commit 3b1f7e8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pdalpra @typesafe-tools @eed3si9n @jsuereth @dwijnand