Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/
---

(Updated Feb. 25, 2015, 7:08 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

The service used so far is broken (doesn't resolve latitude and
longitude). Which means we need to switch the service to unbreak.

This change implements support for the Mozilla Location Service [1].
Advantages are:
* free
* open
* supports wifi based locationing
* provided by our FLOSS friends

[1] https://location.services.mozilla.com/


Diffs
-

  dataengines/geolocation/location_ip.h 
0a7780c1344c0f48416e524249be6fdada6b9bfd 
  dataengines/geolocation/location_ip.cpp 
1be2acd9478fceafa83896c552e7a3177368b194 

Diff: https://git.reviewboard.kde.org/r/122628/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Sebastian Kügler]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76280
---

Ship it!


Others have pointed out some code issues, I haven't found any more.

As to the approach: +2. If memory serves me well, I implemented the hostip 
approach lacking a Free solution at that time. I'm happy that Free service now 
exists, and I'm all for migrating to it. (More so since it's -- again -- 
broken.)

Thanks for looking into it!

- Sebastian Kügler


On Feb. 18, 2015, 2:37 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Feb. 18, 2015, 2:37 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.h 
> 0a7780c1344c0f48416e524249be6fdada6b9bfd 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/
---

(Updated Feb. 18, 2015, 3:37 p.m.)


Review request for Plasma.


Changes
---

* added support for country
* removed the fields which are not set (city, ip)


Repository: plasma-workspace


Description
---

The service used so far is broken (doesn't resolve latitude and
longitude). Which means we need to switch the service to unbreak.

This change implements support for the Mozilla Location Service [1].
Advantages are:
* free
* open
* supports wifi based locationing
* provided by our FLOSS friends

[1] https://location.services.mozilla.com/


Diffs (updated)
-

  dataengines/geolocation/location_ip.h 
0a7780c1344c0f48416e524249be6fdada6b9bfd 
  dataengines/geolocation/location_ip.cpp 
1be2acd9478fceafa83896c552e7a3177368b194 

Diff: https://git.reviewboard.kde.org/r/122628/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Lukáš Tinkl]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76235
---


definitely +1 from me

- Lukáš Tinkl


On Úno. 18, 2015, 1:52 odp., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Úno. 18, 2015, 1:52 odp.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Gräßlin]

> On Feb. 18, 2015, 2:20 p.m., Vishesh Handa wrote:
> > dataengines/geolocation/location_ip.cpp, line 45
> > 
> >
> > Any idea why we have this magic number?

I can only guess: the accuracy returned by mozilla's service for just IP 
address is 5, so I guess it's what was considered a useful accuracy for the 
old service.


> On Feb. 18, 2015, 2:20 p.m., Vishesh Handa wrote:
> > dataengines/geolocation/location_ip.cpp, line 55
> > 
> >
> > It could be possible that 'lat' and 'lng' do not exist in the response. 
> > Perhaps we should be checking for it.

I consider this as unlikely given that it already checks whehter the key 
"location" is available. Assuming well formed data here and if the key is 
missing we get a default value anyway.


> On Feb. 18, 2015, 2:20 p.m., Vishesh Handa wrote:
> > dataengines/geolocation/location_ip.cpp, line 58
> > 
> >
> > This is slightly strange.
> > 
> > 
> > Normally it's given a double value, but otherwise we give it a string. 
> > Why not just not add it?

I based that on the old implementation. If it wasn't present it added it as an 
empty string. I wanted to keep it compatible (as it explicitly mentions the 
ordering mattered in a comment).


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76231
---


On Feb. 18, 2015, 1:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Feb. 18, 2015, 1:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76231
---



dataengines/geolocation/location_ip.cpp


Any idea why we have this magic number?



dataengines/geolocation/location_ip.cpp


It could be possible that 'lat' and 'lng' do not exist in the response. 
Perhaps we should be checking for it.



dataengines/geolocation/location_ip.cpp


This is slightly strange.

Normally it's given a double value, but otherwise we give it a string. Why 
not just not add it?


- Vishesh Handa


On Feb. 18, 2015, 12:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Feb. 18, 2015, 12:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Klapetek]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76229
---


+1 for the approach

- Martin Klapetek


On Feb. 18, 2015, 1:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Feb. 18, 2015, 1:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/#review76228
---



dataengines/geolocation/location_ip.cpp


there's an additional API call to resolve these. If I get enough +1 for the 
general approach, I'll add support for it.



dataengines/geolocation/location_ip.cpp


I'll request an API key for KDE if I get enough +1 for the approach.


- Martin Gräßlin


On Feb. 18, 2015, 1:52 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122628/
> ---
> 
> (Updated Feb. 18, 2015, 1:52 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> The service used so far is broken (doesn't resolve latitude and
> longitude). Which means we need to switch the service to unbreak.
> 
> This change implements support for the Mozilla Location Service [1].
> Advantages are:
> * free
> * open
> * supports wifi based locationing
> * provided by our FLOSS friends
> 
> [1] https://location.services.mozilla.com/
> 
> 
> Diffs
> -
> 
>   dataengines/geolocation/location_ip.cpp 
> 1be2acd9478fceafa83896c552e7a3177368b194 
> 
> Diff: https://git.reviewboard.kde.org/r/122628/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Review Request 122628: [dataengine/geolocation] Switch to datalocation service provided by Mozilla

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122628/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

The service used so far is broken (doesn't resolve latitude and
longitude). Which means we need to switch the service to unbreak.

This change implements support for the Mozilla Location Service [1].
Advantages are:
* free
* open
* supports wifi based locationing
* provided by our FLOSS friends

[1] https://location.services.mozilla.com/


Diffs
-

  dataengines/geolocation/location_ip.cpp 
1be2acd9478fceafa83896c552e7a3177368b194 

Diff: https://git.reviewboard.kde.org/r/122628/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel