CHE-4097: Refactor go agents package structure #4344
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also add comments and cleanup code. Signed-off-by: Alexander Garagatyi <[email protected]>
garagatyi
added
the
kind/task
sleshchenko
reviewed
Mar 7, 2017
| @@ -0,0 +1,2 @@ | |||
| src/main/go/exec-agent/exec-agent | |||
There was a problem hiding this comment.
Are you sure that you should add this to gitignore?
There was a problem hiding this comment.
This is executable of go program. It may be created in development if user manually builds go sources.
Is something wrong with this exclusion?
|
Build finished. Please check console output at $BUILD_URL to view the results. |
|
Build # 2128 - FAILED Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2128/ to view the results. |
voievodin
reviewed
Mar 9, 2017
| // Checks on workspace master if provided by request token is valid and calls ServerHTTP on delegate. | ||
| // Otherwise if UnauthorizedHandler is configured calls ServerHTTP on it. | ||
| // If it is not configured returns 401 with appropriate error message. | ||
| type Handler struct { |
There was a problem hiding this comment.
What if we change it to an internal implementation of http.Handler which can be created by constructing functions like
func NewHandler(endpont, delegate, unauthorizedHandler) http.Handler { ... };
func NewCachingHandler(endpont, delegate, unauthorizedHandler, cache) http.Handler { ... }or event handler builder.
So you can be sure that all the data required for handler is provided + you're be able to pick one of caching or non-caching handler and avoid checks like handler.Cache != nil in runtime.
voievodin
approved these changes
Mar 9, 2017
skabashnyuk
approved these changes
Mar 9, 2017
Signed-off-by: Alexander Garagatyi <[email protected]>
|
@evoevodin I added improvements you've asked |
|
Thx, it looks good to me! |
|
could we ensure golint is not reporting any errors on go source code ? |
|
|
This PR was merged before you commented. So now I can't add fixes until next PR in this area. |
|
Build finished. |
|
Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2144/ |
9 tasks
JPinkney
pushed a commit
to JPinkney/che
that referenced
this pull request
Aug 17, 2017
Also add comments and cleanup code. Signed-off-by: Alexander Garagatyi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Refactor terminal+exec agent to separate common components. This allows to reuse them for creation of install agent and separation of terminal and exec agents.
What issues does this PR fix or reference?
Fixes #4097
Changelog
Refactor golang agents for re-usage of components for creation of other agents.
Release Notes
Not needed, this PR is technical debt and doesn't change anything from user POV.
Docs PR