Skip to content
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

Rewrite maps related code using v2 api #36

Closed
scandindian opened this issue Feb 24, 2017 · 28 comments
Closed

Rewrite maps related code using v2 api #36

scandindian opened this issue Feb 24, 2017 · 28 comments

Comments

@scandindian
Copy link
Contributor

scandindian commented Feb 24, 2017

Although the project is using libraries of earlier versions, the compile SDK version and the versions of the appcompat and design libraries used should be updated to make it compatible with Nougat Devices.

Due to deprecation of few things in the code base it has become tedious to work on it (majorly map related v1 is depreciated).

Even if I try to make changes there will be possibilities that they will cause conflicts. It would be great if we could come up with a solution so it becomes easier for people to set up the project in the Android Studio environment and test.

The possible solution to change the map related code is to remove them to make the other code testable and write them again in v2.

@pa1pal
Copy link
Member

pa1pal commented Feb 24, 2017

@heysadboy Yes you're right. The possible solution is to rewrite the map code in new v2 api because almost everything got changed. Map related files causing errors in the latest sdk version.

@scandindian
Copy link
Contributor Author

@pa1pal Thanks. Can we remove those files to start afresh. They are also hindering the development of other features also due to inability to test the app as whole although I see it passing in Travis.

@pa1pal
Copy link
Member

pa1pal commented Feb 25, 2017

@heysadboy Do not remove the code completely, because we may need this code in future development with latest api to see the features. So see specifically what causes the problem and if you could fix it or else remove it for now.

@anantprsd5
Copy link
Member

@heysadboy @pa1pal MapActivity is from the old, defunct MapsV1 way of integrating Google Maps. Since we cannot get API keys for MapsV1 anymore, so do we have to modify the whole code to use MapsV2?

@pa1pal
Copy link
Member

pa1pal commented Feb 25, 2017

@anantprsd5 Yes we cannot get the api keys for MapsV1 api. And lots of the objects got changed such as GeoPoint, Overlays etc. So from the old code we can get some idea what was implemented earlier and modify it.
The basic maps idea which I get from the old app is that every click of photo it records the Lat and Lang of the location. So those photos on the maps according to the current location. Also @mariobehling can elaborate more about this feature.

@scandindian
Copy link
Contributor Author

scandindian commented Feb 25, 2017

@pa1pal @mariobehling The latitude and longitude of the photo are saved as the metadata nowadays, like instagram uses them automatically at the time of posting the photo. So, instead of recording the data we can use metadata.
I think it is more tedious to update the whole depreciated code in comparison to writing the code from start. Although I won't be removing the code. Same applies for other parts of the project.

@anantprsd5 Yes, we have to modify it in order to use v2.

@anantprsd5
Copy link
Member

@pa1pal @heysadboy Will it not be good if we remove all the code related to maps and start creating issues for the things related with maps one after the other because it is giving errors in all the imports related with maps and it is not possible to install the app in the phone.

@scandindian
Copy link
Contributor Author

@anantprsd5 Please see the second comment, I already suggested that.

@pa1pal
Copy link
Member

pa1pal commented Feb 27, 2017

@anantprsd5 and @heysadboy
My suggestion is to first test the app after removing all maps related functionality one by one, and checking whether it's working after that or not. Also the upgrade of compiled sdk version for which this issue is.
Will move the code to another branch if it works fine and move to the master, feature by feature.
Go ahead 👍
I would also like to get @mariobehling suggestion here.

@abishekvashok
Copy link
Member

Yes I agree with @pa1pal we would need to create a new branch, move it one by one and after I all works fine we would push to master back. Mario sir would be available only after the fossasia summit.

@scandindian
Copy link
Contributor Author

@pa1pal That would be really very helpful.

@anantprsd5
Copy link
Member

@pa1pal I have removed all the maps related functionality and now the gradle builds are running without any issues using sdk version 23. Shall I send a PR for it?

@abishekvashok
Copy link
Member

abishekvashok commented Feb 28, 2017

@anantprsd5 is the code working? If yes pls send it to the master branch if not send it to the development branch which I have created as the plan. It would be better if you could do the same after updating it to target API 25 which is the latest.

@pa1pal
Copy link
Member

pa1pal commented Feb 28, 2017

@anantprsd5 nice effort 👍
I agree with @abhi2424shek please test it after update of compile sdk version, for which this issue is all about. And send PR.

@abishekvashok
Copy link
Member

Now we got the map functionality commented out in the development branch, we would need to rewrite it now.

@DravitLochan
Copy link
Member

I'll be working on maps then.

@jitendra3109
Copy link

@anantprsd5 > I have removed all the maps related functionality and now the gradle builds are running without any issues using sdk version 23. Shall I send a PR for it?
have you send PR ?
Because I still got error related to Error:(3, 101) error: package com.google.android.maps does not exist

@pa1pal
Copy link
Member

pa1pal commented Mar 1, 2017

@jsroyal This improvement is done in this #40 PR and code is in development branch.
We will move the code from development to master feature wise.

@jitendra3109
Copy link

@pa1pal Thanks, I was trying to solve one issue by issue but getting issue one after another.
Okay.
So still not replace all the code should be work fine.
In the development, branch code work fine?

@abishekvashok
Copy link
Member

@jsroyal The maps are needed to get the app working bug less. It is depreciated so we commented out the map code and put it in the development branch. So when we should fix the maps by rewriting the code in v2 and then make a pr to the master branch.

@jitendra3109
Copy link

@Abhi2424shek Problem is master branch code not able to build the project because getting an error about the map API so my suggestion is put the code in master branch at least base code where can anybody start project build. Want more contribute.
As of now, I clone the development branch and building project.

@abishekvashok
Copy link
Member

@jsroyal the map API is depreciated, we need to rewrite it to build the app, inorder not to risk we created a branch. Master branch has the code which was modified in the development branch to comment out the map codes :) you can help by fixing parts of the map

@abishekvashok abishekvashok changed the title Compile SDK version and the depreciated code Rewrite maps related code using v2 api Mar 2, 2017
@anantprsd5
Copy link
Member

@DravitLochan Hey, Are you working on this issue?

@DravitLochan
Copy link
Member

I am really very sorry, but i was busy in susi-android. @pa1pal can you please assign someone else to this?

@abishekvashok
Copy link
Member

Ok so, who wants this? Anyone?

@scandindian
Copy link
Contributor Author

@Abhi2424shek Can I work on this?

@abishekvashok
Copy link
Member

Sure @heysadboy

This was referenced Mar 9, 2017
@abishekvashok
Copy link
Member

Is'nt this solved in #135 if not pls open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants