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

Cleaned up AIMSICDDbAdapter implementation #640

Merged
merged 1 commit into from
Nov 19, 2015
Merged

Cleaned up AIMSICDDbAdapter implementation #640

merged 1 commit into from
Nov 19, 2015

Conversation

smarek
Copy link
Member

@smarek smarek commented Nov 17, 2015

  • Formatted code and comments
  • Replaced hard-coded database path with Context related path (it's the same, but allows repackaging and dev package extensions, etc..)
  • Finalized for performance purposes
  • Fixed few typos in logs
  • Removed unused field and class variables (mostly commented out)
  • Removed unnecessary .toString() calls
  • Avoided NPE on few Cursor usages
  • Updated CommandResult and RequestTask according to variable name changed

@smarek
Copy link
Member Author

smarek commented Nov 17, 2015

Also what's up with that excessive comment-based discussions? That's real mess, and I want to remove them all once we get to fix the DB layer implementation to provide objects and downgrade-upgrade procedures.

@SecUpwN
Copy link
Member

SecUpwN commented Nov 17, 2015

Also what's up with that excessive comment-based discussions?

One of the things E:V:A was fighting for, was to keep comments within the code so that developers could possibly continue already started code and understand existing code as well. But if you ask me, I think that we should only keep valuable code comments that are inevitable to understand difficult parts and make damn sure that code comments are following a code comment standard that does not clutter our code. I'd appreciate a fresh Issue for the code comment cleanup so that we can link PRs for that, too.

@He3556
Copy link
Collaborator

He3556 commented Nov 17, 2015

i am asking for a central documentation about the code, for a long time. So we all knew where the important things happen (detection) and get a overview about the classes. But it seems like nobody is interested. New developers might like it, when they start to work in this project.

@smarek
Copy link
Member Author

smarek commented Nov 17, 2015

@He3556 I'd like it as well, do you have any specific idea about that? Or if you haven't opened issue already, that would be cool 👍 Thanks

@ghost
Copy link

ghost commented Nov 17, 2015

Still a newbie when it comes to database and all comments where for EVA to understand what was happen as it was still in dev, I was kinda being pushed to finished this fast as it was holding up the project development since it was tied to every other function. Got burnt out from coding this and left a good base to work off besides the errors and comments.

@smarek smarek added the bug label Nov 17, 2015
@smarek smarek added this to the v0.1.37-alpha milestone Nov 17, 2015
@smarek smarek self-assigned this Nov 17, 2015
@SecUpwN
Copy link
Member

SecUpwN commented Nov 17, 2015

i am asking for a central documentation about the code, for a long time. So we all knew where the important things happen (detection) and get a overview about the classes. But it seems like nobody is interested. New developers might like it, when they start to work in this project.

@He3556, wasn't General Overview and Technical Overview meant for documentation?

@smarek
Copy link
Member Author

smarek commented Nov 19, 2015

@SecUpwN this PR doesn't really touch the documentation, the question was reaction to observed state of code, so I don't think we need to stay put here

SecUpwN added a commit that referenced this pull request Nov 19, 2015
Cleaned up AIMSICDDbAdapter implementation
@SecUpwN SecUpwN merged commit e8b70b5 into CellularPrivacy:development Nov 19, 2015
@smarek smarek deleted the class-cleanup-3 branch November 19, 2015 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants