Skip to content

Conversation

@ibsoln
Copy link
Contributor

@ibsoln ibsoln commented Apr 2, 2020

@ibsoln ibsoln requested review from a team and bmeike April 2, 2020 08:27
@bmeike
Copy link
Contributor

bmeike commented Apr 2, 2020

It looks good to me. I have the following niggles:

  1. "Comment out" is very specific. I think it might be good enough to say "Unnecessary if none of your source code is Kotlin"
  2. Consistent code formatting would be nice. We have
kotlinOptions { jvmTarget = '1.8' }

and...

compileOptions
{

and...

    maven {
        url "https://mobile.maven.couchbase.com/maven2/dev/"
    }

I suggest, instead:

kotlinOptions { jvmTarget = '1.8' }

compileOptions {
...

maven { url "https://mobile.maven.couchbase.com/maven2/dev/" }
  1. Finally, it would be nice to be consistent about the way sections are displayed. The point describing what gets included in the android section does not show the android section, and is not indented so that it is clear that it goes into the section. The next two points, Update Carthage instruction #2 & encryption specification #3 do show the including section but do not allude to the fact that the sections will, almost certainly, contain other things, needed by the application program. Also, note, FWIW, that there is nothing about CBLite that requires either google() or jcenter().

These are just niggles. Adopt them or not as you see fit.

@ibsoln
Copy link
Contributor Author

ibsoln commented Apr 7, 2020

Content ready to merge. https://couchbase.slack.com/archives/DS1QC61RV/p1585931768001600
Needs an and-1 approval @couchbase/documentation-writers

Copy link
Contributor

@bmeike bmeike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@ibsoln ibsoln merged commit 913da54 into couchbase:release/2.7 Apr 8, 2020
@ibsoln ibsoln deleted the DOC-6448-Set-java-compatibility-version-in-build-gradle branch April 8, 2020 14:31
ibsoln added a commit to ibsoln/docs-couchbase-lite that referenced this pull request Aug 11, 2021
* DOC-6448-Set-java-compatibility-version-in-build-gradle

https://issues.couchbase.com/browse/DOC-6448

* DOC-6448-Set-java-compatibility-version-in-build-gradle

https://issues.couchbase.com/browse/DOC-6448

Update in line with review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants