Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Small refactoring of interpose/localbypass/clienturl #474

Merged
merged 3 commits into from
Sep 23, 2020
Merged

Small refactoring of interpose/localbypass/clienturl #474

merged 3 commits into from
Sep 23, 2020

Conversation

denis-tingaikin
Copy link
Member

@denis-tingaikin denis-tingaikin commented Sep 23, 2020 •
edited
Loading

Signed-off-by: Denis Tingajkin denis.tingajkin@xored.com

Motivation

  1. Moved clienturl to tools, that reduced lines of code and entities count.
  2. Added clienturl.Map and reused it in interpose and localbypass chain elements.

@denis-tingaikin denis-tingaikin mentioned this pull request Sep 23, 2020
Merged
@denis-tingaikin
refactor interpose, localbypass chain elements
4d46f19 
Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin denis-tingaikin force-pushed the refactor_interpose_chain_element branch from 496fd51 to 4d46f19 Compare September 23, 2020 08:39
@denis-tingaikin denis-tingaikin changed the title Refactor interpose, localbypass chain elements Small refactoring of interpose/localbypass/clienturl Sep 23, 2020
@edwarnicke
Copy link
Member

edwarnicke commented Sep 23, 2020 •
edited
Loading

Overall good idea. I'd suggest though that:

  1. The context bit go into pkg/tools/clienturlctx
  2. The Map bit go into pkg/tools/stringurl (so referencing it is stringurl.Map)

This way you wind up with clean pkg naming that doesn't collide with clienturl chain elements.

@denis-tingaikin
apply review comments
181aae6 
Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin
Copy link
Member Author

@edwarnicke awesome idea, fixed

@edwarnicke
Copy link
Member

@edwarnicke awesome idea, fixed

Ah.. I updated my comment to suggest stringurl not clienturlmap as package names there ... makes things a little more generic, in case there's a need for stringurl outside the clienturl context :)

@denis-tingaikin
rename clienturlmap to stringurl
c5d8afd 
Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>
@denis-tingaikin
Copy link
Member Author

denis-tingaikin commented Sep 23, 2020 •
edited
Loading

@edwarnicke Indeed stringurl looks better and potentially can be reused. Fixed.

@edwarnicke edwarnicke merged commit 3b08d97 into networkservicemesh:master Sep 23, 2020
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Sep 23, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
183b71e 
…k@master networkservicemesh/sdk#474

networkservicemesh/sdk PR link: networkservicemesh/sdk#474

networkservicemesh/sdk commit message:
commit 3b08d97e38650d6e85de383b395431a08c99a9eb
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 23 22:46:50 2020 +0700

    Small refactoring of interpose/localbypass/clienturl  (#474)

    * refactor interpose, localbypass chain elements

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * rename clienturlmap to stringurl

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Sep 23, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
ca741c8 
…k@master networkservicemesh/sdk#474

networkservicemesh/sdk PR link: networkservicemesh/sdk#474

networkservicemesh/sdk commit message:
commit 3b08d97e38650d6e85de383b395431a08c99a9eb
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 23 22:46:50 2020 +0700

    Small refactoring of interpose/localbypass/clienturl  (#474)

    * refactor interpose, localbypass chain elements

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * rename clienturlmap to stringurl

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Sep 23, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
ff43b13 
…k@master networkservicemesh/sdk#474

networkservicemesh/sdk PR link: networkservicemesh/sdk#474

networkservicemesh/sdk commit message:
commit 3b08d97e38650d6e85de383b395431a08c99a9eb
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 23 22:46:50 2020 +0700

    Small refactoring of interpose/localbypass/clienturl  (#474)

    * refactor interpose, localbypass chain elements

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * rename clienturlmap to stringurl

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Sep 23, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
6ddf557 
…k@master networkservicemesh/sdk#474

networkservicemesh/sdk PR link: networkservicemesh/sdk#474

networkservicemesh/sdk commit message:
commit 3b08d97e38650d6e85de383b395431a08c99a9eb
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 23 22:46:50 2020 +0700

    Small refactoring of interpose/localbypass/clienturl  (#474)

    * refactor interpose, localbypass chain elements

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * rename clienturlmap to stringurl

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-vppagent that referenced this pull request Sep 23, 2020
Update go.mod and go.sum to latest version from networkservicemesh/sd…
c8b8b72 
…k@master networkservicemesh/sdk#474

networkservicemesh/sdk PR link: networkservicemesh/sdk#474

networkservicemesh/sdk commit message:
commit 3b08d97e38650d6e85de383b395431a08c99a9eb
Author: Denis Tingaikin <49399980+denis-tingajkin@users.noreply.github.com>
Date:   Wed Sep 23 22:46:50 2020 +0700

    Small refactoring of interpose/localbypass/clienturl  (#474)

    * refactor interpose, localbypass chain elements

    Signed-off-by: Denis Tingajkin <denis.tingajkin@xored.com>

    * apply review comments

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

    * rename clienturlmap to stringurl

    Signed-off-by: denis-tingajkin <denis.tingajkin@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haiodo haiodo Awaiting requested review from haiodo

@edwarnicke edwarnicke Awaiting requested review from edwarnicke

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@denis-tingaikin @edwarnicke