Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

proxy bridge#84

Merged
effervescentia merged 1 commit into
developgroupby/api-javascript:developfrom
EN-410groupby/api-javascript:EN-410Copy head branch name to clipboard
May 22, 2018
Merged

proxy bridge#84
effervescentia merged 1 commit into
developgroupby/api-javascript:developfrom
EN-410groupby/api-javascript:EN-410Copy head branch name to clipboard

Conversation

@cnradich

Copy link
Copy Markdown
Contributor

No description provided.

@codecov-io

codecov-io commented May 14, 2018

Copy link
Copy Markdown

Codecov Report

Merging #84 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop      #84   +/-   ##
========================================
  Coverage    99.17%   99.17%           
========================================
  Files            3        3           
  Lines          241      241           
  Branches        21       22    +1     
========================================
  Hits           239      239           
  Partials         2        2
Impacted Files Coverage Δ
src/core/bridge.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab47470...4131045. Read the comment docs.

Comment thread src/core/bridge.ts Outdated
}
}

export class ProxyBridge extends AbstractBridge {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rather than add a whole new class, I would simply make baseUrl an optional property of BridgeConfig
then in the constructor()s of BrowserBridge and CloudBridge you can just have

export interface BridgeConfig {
   timeout?: number;
   baseUrl?: string;
}

// in constructor()

this.baseUrl =  config.baseUrl ||`${scheme}://${customerId}-cors.groupbycloud.com${port}/api/v1`;

// and

this.baseUrl = config.baseUrl || `https://${customerId}.groupbycloud.com:443/api/v1`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or even proxyUrl so that you don't have to change your other PR much :P

@effervescentia effervescentia merged commit 77cfa37 into develop May 22, 2018
@effervescentia effervescentia deleted the EN-410 branch May 22, 2018 18:29
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.