-
Notifications
You must be signed in to change notification settings - Fork 99
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
Expose Location information #239
Expose Location information #239
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #239 +/- ##
==========================================
- Coverage 76.74% 76.09% -0.65%
==========================================
Files 71 73 +2
Lines 2283 2334 +51
Branches 230 234 +4
==========================================
+ Hits 1752 1776 +24
- Misses 468 495 +27
Partials 63 63
|
src/CarbonAware.CLI/test/integrationTests/Commands/Location/LocationCommandTests.cs
Outdated
Show resolved
Hide resolved
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.
This is a cool solution, thanks for the taking the time to do it all. I had one quick question about how the user will know about the duplicated key situation
e24ddc2
to
6aadfc0
Compare
This is awesome, let me know when it's ready for us to review. We had been looking at this for a while and it came up in the hackathon to document on the website the locations, but we had decided that a service would be better to get the data live. For these named locations however, should we also be doing forecast? |
Co-authored-by: Priti <[email protected]>
6aadfc0
to
7ffc45e
Compare
Hi @vaughanknight, thank you for pointing this out. The WebApi route should be |
78f0d1e
to
0942c07
Compare
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.
Looks great, thanks for the comment
@vaughanknight ready for review |
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.
Very happy with this one. Thank you very much. We may look a changing the response type maybe but I want to get it into dev as we feel it's best if we get some people using it and get feedback before it goes into main. Thanks again!
Issue Number: (microsoft#229, microsoft#214, #175)
Summary
Expose Locations data through all the 'user's' interfaces (WebApi, CLI, GSF SDK)
Changes
/locations
caw locations
ILocationHandler
Checklist
Are there API Changes?
If yes, what are the expected API Changes? Please link to an API-Comparison workflow with the API Diff.
/locations
caw locations
ILocationHandler.GetLocationsAsync
Close #175