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 Android documentation to fit Android Studio/Gradle and SQLDroid better #65

Closed
wants to merge 2 commits into from
Closed

Conversation

@jhannes
Copy link

@jhannes jhannes commented Mar 24, 2016

<pre class="prettyprint">import org.flywaydb.core.Flyway;
import org.flywaydb.core.api.android.ContextHolder;
import android.database.sqlite.SQLiteDatabase;
import org.sqldroid.DroidDataSource;

This comment has been minimized.

@axelfontaine

axelfontaine Apr 22, 2016
Contributor

Why the change in Driver class? Is the old one broken?

This comment has been minimized.

@jhannes

jhannes Apr 23, 2016
Author

Both will work. I suggest this variant because it makes explicit the connection between the database and Flyway. It helped me explain to colleagues what is going on. Which variant to prefer depends on how you want to use SqlDroid further.

<li
{% if page.pill == 'overview' %} class="active" {% endif %}>
<a href="/documentation/api#spring">Android</a></li>
<li

This comment has been minimized.

@axelfontaine

axelfontaine Apr 22, 2016
Contributor

This changes the 1-1 mapping between navigation and pages and breaks the consistency of the convention used for the rest of the site.

This comment has been minimized.

@jhannes

jhannes Apr 23, 2016
Author

I think the API page is larger than the others, and it covers more variants.

NB: I just spotted a typo in my patch: the link to Spring is labeled "Android"

<a href="/documentation/api#android">API (Android)</a></li>
<li
{% if page.menu == 'api' %} class="active" {% endif %}>
<a href="/documentation/api#spring">API (Spring)</a></li>
<li

This comment has been minimized.

@axelfontaine

axelfontaine Apr 22, 2016
Contributor

This changes the 1-1 mapping between navigation and pages and breaks the consistency of the convention used for the rest of the site.

This comment has been minimized.

@jhannes

jhannes Apr 23, 2016
Author

I was conflicted about this. I found myself searching at random for the android info many times, however. It would be nice if it was more directly accessible.

For me, the Java API is the supremely important part of the doc. The other pages, I have barely read.

@@ -88,22 +88,45 @@
instance (both locally and on AWS), Flyway will automatically use the JDBC url, user and password
<a href="https://boxfuse.com/docs/databases#envvars">provided by Boxfuse</a>.</div>

<h2>Programmatic Configuration (Android)</h2>
<h2 id="android">Android Programmatic Configuration (Android Studio)</h2>

This comment has been minimized.

@axelfontaine

axelfontaine Apr 22, 2016
Contributor

Is this really Android Studio-specific?

This comment has been minimized.

@jhannes

jhannes Apr 23, 2016
Author

Not really. But Android development these days is almost almost documented in terms of Android Studio. Technically, the description only relies on Gradle (because of the definition of deps). But there is an old Gradle-way and a new Gradle-way (which came with Android Studio). I think the need to define assets is different in the old way.

In short: the details in this description are necessary and sufficient for up-to-date newly created android projects, but details are different for older projects.

@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented Apr 22, 2016

Hi Johannes,

Thank you very much for this and apologies for the late review. Please find my comments inline.

Cheers
Axel

axelfontaine added a commit that referenced this pull request May 5, 2016
@axelfontaine
Copy link
Contributor

@axelfontaine axelfontaine commented May 5, 2016

Thanks Johannes! I ended up pulling in the changes in content, without the changes in the navigation.

Cheers
Axel

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

Successfully merging this pull request may close these issues.

None yet

2 participants