Skip to content

fix: replace hostname with host in admin config#876

Merged
XVincentX merged 4 commits intoExpressGateway:masterfrom
ravikp7:fix-hostname-consistency
Mar 13, 2019
Merged

fix: replace hostname with host in admin config#876
XVincentX merged 4 commits intoExpressGateway:masterfrom
ravikp7:fix-hostname-consistency

Conversation

@ravikp7
Copy link
Copy Markdown
Contributor

@ravikp7 ravikp7 commented Mar 11, 2019

  • improves consistency with apiEndpoints config
  • generators now use host in admin config
  • hostname is still supported, but warning is shown

fixes #432

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2019 •
edited
Loading

Codecov Report

Merging #876 into master will decrease coverage by 0.1%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #876      +/-   ##
==========================================
- Coverage   89.25%   89.15%   -0.11%     
==========================================
  Files         135      135              
  Lines        3649     3652       +3     
==========================================
- Hits         3257     3256       -1     
- Misses        392      396       +4
Impacted Files Coverage Δ
bin/eg-generator.js 87.17% <0%> (ø) ⬆️
lib/rest/index.js 91.11% <80%> (-1.75%) ⬇️
lib/config/config.js 90.21% <0%> (-3.27%) ⬇️

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 a5942b0...2345555. Read the comment docs.

XVincentX
XVincentX requested changes Mar 12, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@XVincentX XVincentX left a comment

Choose a reason for hiding this comment

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

Nice start! This is going to require some work though and I'd appreciate if you could put at least one test, although I can live without this.

@ravikp7
Copy link
Copy Markdown
Contributor Author

ravikp7 commented Mar 12, 2019

I'm not sure of where to put tests or what to test for the changes made in this PR.
Can you please guide me on this?

XVincentX
XVincentX reviewed Mar 12, 2019
View reviewed changes
@XVincentX
Copy link
Copy Markdown
Member

XVincentX commented Mar 12, 2019

I'm not sure of where to put tests or what to test for the changes made in this PR.

Good point. Let's first make sure the code is ready and then CodeCov will guide us in the part that needs to be covered.

It's not exactly that mandatory to test this stuff (I can live with it) but it'd be great to onboard you appropriately :)

Keep up the good work!

@ravikp7 ravikp7 force-pushed the fix-hostname-consistency branch 3 times, most recently from 116ccc4 to bdf2ef7 Compare March 12, 2019 22:19
@XVincentX XVincentX force-pushed the fix-hostname-consistency branch 3 times, most recently from 9bba851 to cfc81da Compare March 13, 2019 15:47
@ravikp7 @XVincentX
fix: replace hostname with host in admin config
69185dd 
- improves consistency with apiEndpoints config
- generators now use host in admin config
- hostname is still supported, but warning is shown

fixes ExpressGateway#432
@XVincentX XVincentX force-pushed the fix-hostname-consistency branch 3 times, most recently from 5097501 to 365c1e2 Compare March 13, 2019 15:55
@XVincentX XVincentX force-pushed the fix-hostname-consistency branch from 365c1e2 to 5c19ba2 Compare March 13, 2019 15:56
@XVincentX XVincentX force-pushed the fix-hostname-consistency branch from 513e77a to 2345555 Compare March 13, 2019 16:10
@XVincentX
Copy link
Copy Markdown
Member

XVincentX commented Mar 13, 2019

I also went ahead and checked what's up with the tests and the untested part do not concern this PR. Good to go! 🚀

@XVincentX XVincentX merged commit 8cb9ab9 into ExpressGateway:master Mar 13, 2019
@ravikp7 ravikp7 deleted the fix-hostname-consistency branch March 13, 2019 17:29
@snyk-bot snyk-bot mentioned this pull request Jan 24, 2020
Open
gatherchou pushed a commit to yilu-tech/express-gateway that referenced this pull request Jul 29, 2021
@XVincentX
Merge pull request ExpressGateway#876 from ravikp7/fix-hostname-consi…
2fc2dec 
…stency

fix: replace hostname with host in admin config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@XVincentX XVincentX XVincentX approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

host versus hostname: need consistency

2 participants

@ravikp7 @XVincentX