Add Slice Sink #47

thomaspoignant · 2020-04-06T15:08:42Z

It resolve this issue: #45

It resolve this issue: bitfield#45

bitfield

Nice job!

README.md

examples/slice/main.go

examples/visitors/go.sum

sinks.go

sinks_test.go

Co-Authored-By: John Arundel <john@bitfieldconsulting.com>

thomaspoignant · 2020-04-27T19:15:44Z

@bitfield I have made the requested changes.
I hope this is what you have in mind?

bitfield

Good work so far! There's a few outstanding comments that haven't yet been addressed, and I've added a couple of new ones. Thanks for working on this 😄

.gitignore

sinks_test.go

README.md

examples/slice/main.go

sinks.go

Co-authored-by: John Arundel <john@bitfieldconsulting.com>

bitfield · 2020-05-31T10:05:47Z

Hi @thomaspoignant, how are you getting on? Would you like any help with this? Are you ready for another review yet?

thomaspoignant · 2020-05-31T11:53:36Z

Hi @bitfield sorry I was not able to work on this, I will try to work on this next week.

I am really sorry about that delay.

bitfield · 2020-06-01T14:52:59Z

Don't worry about it! I don't get nearly as much time as I'd like to work on this project either. If you'd like some help finishing this off, just let me know. Otherwise, there's no hurry.

thomaspoignant · 2020-06-02T09:04:43Z

@bitfield I have made almost all the changes you requested.
The last thing is how we iterate over the string, it is not clear to me if we should use String() or EachLine() to iterate.

bitfield · 2020-06-02T09:33:51Z

If we can use EachLine(), that's probably more efficient.

thomaspoignant · 2020-06-02T09:51:22Z

@bitfield Should be good now, it use the version with EachLine().

bitfield

Awesome work!

README.md

examples/slice/main.go

pipes_test.go

sinks.go

sinks_test.go

thomaspoignant · 2020-06-02T13:24:17Z

Sorry about all these order problems, it should be fine now.

thomaspoignant · 2020-06-29T08:23:44Z

@bitfield do you need more work on this PR?

bitfield

Looking good! Sorry about the long delay in review.

examples/slice/main.go

sinks_test.go

thomaspoignant · 2020-06-30T13:58:22Z

I send the changes if you could have a look.

bitfield

Excellent! I think this is nearly ready to merge.

sinks_test.go

examples/slice/main.go

thomaspoignant · 2020-07-02T07:37:09Z

There is an example file + I have changed the order in comparison.

bitfield · 2020-07-02T12:18:50Z

Great job @thomaspoignant! Thanks a lot for the help. I've given it a final pass for polish, and merged this as v0.18.0. Enjoy!

thomaspoignant added 2 commits April 6, 2020 16:53

Adding Slice Sink

f74894f

It resolve this issue: bitfield#45

Add test with empty pipe

b928492

thomaspoignant mentioned this pull request Apr 9, 2020

Add 'Slice()' sink method to get pipe contents as a slice of string #45

Closed

bitfield requested changes Apr 11, 2020

View reviewed changes

thomaspoignant and others added 6 commits April 12, 2020 17:58

Apply suggestions from code review

62284e7

Co-Authored-By: John Arundel <john@bitfieldconsulting.com>

Not using type as variable name

7f35d71

Using cmp to deep compare

455d7ad

Not testing ListFiles in the test method for SliceSink

8f9a5a5

Remove examples/visitors/go.sum who should not be commited

e3c8ba7

An empty pipe should return an empty Slice

92fd314

thomaspoignant force-pushed the feature/add_slice_sink branch from efc8d16 to 92fd314 Compare April 15, 2020 16:48

Better example for Slice

fee0b63

thomaspoignant requested a review from bitfield April 15, 2020 17:04

bitfield requested changes Apr 30, 2020

View reviewed changes

Ignore all go.sum

2444194

Co-authored-by: John Arundel <john@bitfieldconsulting.com>

thomaspoignant added 9 commits June 2, 2020 10:24

use cmp.diff when printing error

f51d95f

Remove comma in doc

8fbab2d

Move Slice after SHA256

f88b6c6

Edit comments

5cbbe48

Adding trailing blank line:

0bfd472

Single line with embeded new lines

bafc161

Code style

312d538

Range over pids

190a6c6

Code style

6302de1

Using EachLine to split on each lines

0c994d3

thomaspoignant requested a review from bitfield June 2, 2020 09:50

bitfield requested changes Jun 2, 2020

View reviewed changes

thomaspoignant added 6 commits June 2, 2020 14:57

Slice is supposed to be after SHA256

e8c308b

Change order of params in cmp.Equal

8853793

Better explanation in test

c366acf

typo

5f5a674

Better example who don't use system specific command

0a75bc1

still an order problem

930bd15

thomaspoignant requested a review from bitfield June 2, 2020 13:10

Order problem

fb12d43

bitfield requested changes Jun 30, 2020

View reviewed changes

examples/slice/main.go Show resolved Hide resolved

sinks_test.go Outdated Show resolved Hide resolved

Thomas Poignant added 2 commits June 30, 2020 15:47

Add comment to the example for slice

590add3

Move the test to a t.Run format

81911bc

thomaspoignant requested a review from bitfield June 30, 2020 13:58

bitfield requested changes Jul 1, 2020

View reviewed changes

sinks_test.go Outdated Show resolved Hide resolved

examples/slice/main.go Outdated Show resolved Hide resolved

Thomas Poignant added 2 commits July 1, 2020 17:21

Change order for cmp.Equal

992c251

Add example file

dcca152

thomaspoignant requested a review from bitfield July 2, 2020 07:36

Tweaks to SliceSink

881413b

bitfield merged commit 9847d19 into bitfield:master Jul 2, 2020

Uh oh!

Add Slice Sink #47

Add Slice Sink #47

Uh oh!

Conversation

thomaspoignant commented Apr 6, 2020

Uh oh!

bitfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomaspoignant commented Apr 27, 2020

Uh oh!

bitfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bitfield commented May 31, 2020

Uh oh!

thomaspoignant commented May 31, 2020

Uh oh!

bitfield commented Jun 1, 2020

Uh oh!

thomaspoignant commented Jun 2, 2020

Uh oh!

bitfield commented Jun 2, 2020

Uh oh!

thomaspoignant commented Jun 2, 2020

Uh oh!

bitfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thomaspoignant commented Jun 2, 2020

Uh oh!

thomaspoignant commented Jun 29, 2020

Uh oh!

bitfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thomaspoignant commented Jun 30, 2020

Uh oh!

bitfield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thomaspoignant commented Jul 2, 2020

Uh oh!

bitfield commented Jul 2, 2020

Uh oh!

Uh oh!