search

google / blockly-android

Blockly for Android
Apache License 2.0
673 stars 209 forks source link

Starter coder for upcoming Codelab #626

Closed AnmAtAnm closed 7 years ago

AnmAtAnm commented 7 years ago

Adding a new project directory Codelab with subproject codelab-starter. Implemented a first attempt at the starter code mirroring the iOS codelab.

This change is Reviewable

vicng commented 7 years ago

Minor nits, but :lgtm:

Review status: 0 of 33 files reviewed at latest revision, 3 unresolved discussions.

Codelab/codelab-starter/src/main/AndroidManifest.xml, line 6 at r1 (raw file):


    <application
        android:allowBackup="true"

Add min version?

Codelab/codelab-starter/src/main/java/com/google/blockly/codelab/MainActivity.java, line 28 at r1 (raw file):

    private int mMode = MODE_PLAY;

    private ActionBar mActionBar;

Add comments explaining these variables, or at least a comment for the group.

Codelab/codelab-starter/src/main/java/com/google/blockly/codelab/MainActivity.java, line 57 at r1 (raw file):

    }

    /* package */ void startSelectForEdit() {

Is / package / for readability? Never seen that before.

Comments from Reviewable

AnmAtAnm commented 7 years ago

Review status: 0 of 33 files reviewed at latest revision, 3 unresolved discussions.

Codelab/codelab-starter/src/main/java/com/google/blockly/codelab/MainActivity.java, line 28 at r1 (raw file):

Previously, vicng wrote…
Add comments explaining these variables, or at least a comment for the group.

Removed the EXTRA and MODE constants, as they are no longer used.

Codelab/codelab-starter/src/main/java/com/google/blockly/codelab/MainActivity.java, line 57 at r1 (raw file):

Previously, vicng wrote…
Is /* package */ for readability? Never seen that before.

Yup. Just making something explicit that was previously implicit. Also shows it was intentional (instead of lazy).

Comments from Reviewable