Skip to content

Conversation

smacker
Copy link
Contributor

@smacker smacker commented Dec 11, 2018

I have rewritten concurrentRequest function using channels to be able to
exit as soon as the first analyzer failed.

commentList is also replaced with channel because it's weird to mix
sync package and channels in one function.

fix: #409

Signed-off-by: Maxim Sukharev max@smacker.ru

I have rewritten concurrentRequest function using channels to be able to
exit as soon as the first analyzer failed.

commentList is also replaced with channel because it's weird to mix
sync package and channels in one function.

fix: #409

Signed-off-by: Maxim Sukharev <max@smacker.ru>
@@ -51,7 +54,7 @@ func NewServer(
reviewTimeout time.Duration,
pushTimeout time.Duration,
) *Server {
return &Server{p, fileGetter, analyzers, eventOp, commentOp, reviewTimeout, pushTimeout}
return &Server{false, p, fileGetter, analyzers, eventOp, commentOp, reviewTimeout, pushTimeout}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm late in the discussion... but... I'd prefer not to use positional arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry David but it's unrelated to this PR. It uses the same style that was used before. We can fix it in another PR later if you want.

Signed-off-by: Maxim Sukharev <max@smacker.ru>
Copy link
Contributor

@carlosms carlosms left a comment

Choose a reason for hiding this comment

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

👍

Signed-off-by: Maxim Sukharev <max@smacker.ru>
@smacker
Copy link
Contributor Author

smacker commented Dec 13, 2018

I updated the failing test also.

@smacker smacker merged commit e4cbebb into src-d:master Dec 13, 2018
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Dec 28, 2018


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Dec 28, 2018


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Jan 8, 2019


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Jan 8, 2019


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Jan 8, 2019


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Jan 14, 2019


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
dpordomingo added a commit to dpordomingo/lookout that referenced this pull request Jan 15, 2019


Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change lookout-sdk return code if analysis failed
3 participants