-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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.
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
Handle CORS requests #6289
Handle CORS requests #6289
Conversation
Signed-off-by: Tamal Saha <tamal@appscode.com>
Codecov Report
@@ Coverage Diff @@
## master #6289 +/- ##
=========================================
+ Coverage 41.38% 41.4% +0.01%
=========================================
Files 440 441 +1
Lines 59703 59736 +33
=========================================
+ Hits 24709 24734 +25
- Misses 31758 31762 +4
- Partials 3236 3240 +4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more items to be addressed.
@tamalsaha could you resolve git conflicts, update to use go modules, add tests, and update per feedback? If you do not have any available time to do so please let us know and I can take over this PR to make the required changes. |
@techknowlogick, please feel free to make changes as you see fit. My bandwidth is tight for next few weeks. Thanks! |
go.mod
Outdated
@@ -136,5 +137,6 @@ require ( | |||
|
|||
replace ( | |||
github.com/denisenkom/go-mssqldb v0.0.0-20181014144952-4e0d7dc8888f => github.com/denisenkom/go-mssqldb v0.0.0-20161128230840-e32ca5036449 | |||
github.com/go-macaron/cors v0.0.0-20190309005821-1733aeedd68a => github.com/tamalsaha/cors v0.0.0-20190309005821-1733aeedd68a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this replace needed? https://github.com/go-macaron/cors is up-todate with github.com/tamalsaha/cors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See: go-macaron/cors#1
it fails otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@techknowlogick , I just updated go-macaron/cors@6fd6a9b . Please revendor . This should be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fix 😄
@techknowlogick , do you plan to make any more changes? I wonder when this will be merged. |
@tamalsaha my changes are done, and only one other review is needed. Once a 2nd LG-TM is given then this can be merged. |
Is there anything we can do to accelerate a 2nd LG-TM? |
Please resolve the conflicts. |
# Conflicts: # Gopkg.lock # routers/api/v1/api.go
@@ -135,6 +136,6 @@ require ( | |||
) | |||
|
|||
replace ( | |||
github.com/denisenkom/go-mssqldb v0.0.0-20181014144952-4e0d7dc8888f => github.com/denisenkom/go-mssqldb v0.0.0-20161128230840-e32ca5036449 | |||
github.com/go-sql-driver/mysql v1.4.0 => github.com/go-sql-driver/mysql v0.0.0-20181218123637-c45f530f8e7f | |||
github.com/denisenkom/go-mssqldb => github.com/denisenkom/go-mssqldb v0.0.0-20161128230840-e32ca5036449 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am following the syntax from https://github.com/golang/go/wiki/Modules#when-should-i-use-the-replace-directive
@lunny , I have fixed the conflicts. PTAL. |
Fixes #5724