Updated README for Label dashboard profile sept 2023 #1037

niccolopaganini · 2023-09-23T19:56:23Z

Merging to master branch as @shankari merged fixed the issue regarding the merge and it makes more sense to the master branch. I will be closing #1030 after this goes up.

Changes:

Restructured the README for clarity and better flow (in my opinion)
- Added a contents section
- Added markdown anchors
- Added screenshot for final output
Updated certain links
Updated version number
Addressed previous concerns
- build version/ type of build location
- Verified method of build

@JGreenlee

1. Updated README like @JGreenlee mentioned to successfully build 2. Maintaining current version 3. Will update again after updating JDK in this intel machine

…ated certain resources

- Added hyperlink to the title - Added output screenshot

- Updating markdown anchors

Updating markdown anchors as 4,5, & 6 weren't working

- touch up - fixed non-working markdown anchors

shankari

I also don't see the CI for the SDK install in the README
e-mission/e-mission-docs#958 (comment)

shankari · 2023-09-23T20:55:35Z

README.md


-:sparkles: This has now been upgraded to cordova android@9.0.0 and iOS@6.0.1 ([details](https://github.com/e-mission/e-mission-docs/issues/554)). It has also been upgraded to [android API 29](https://github.com/e-mission/e-mission-phone/pull/707/), [cordova-lib@10.0.0 and the most recent node and npm versions](https://github.com/e-mission/e-mission-phone/pull/708)It also now supports CI, so we should not have any build issues in the future. The limitations from the [previous upgrade](https://github.com/e-mission/e-mission-docs/issues/519) have all been resolved. This should be ready to build out of the box, after all the configuration files are changed.
+:sparkles: This has now been upgraded to cordova android@12.0.0 and iOS@6.2.0. It has also been upgraded to [android API 33 and the latest iOS versions](https://github.com/e-mission/e-mission-docs/issues/934), [cordova-lib@10.0.0 and the most recent node and npm versions](<insert link>). It also now supports CI, so we should not have any build issues in the future. __This should be ready to build out of the box.__


Good job updating the versions! There is still an <insert_link> which you need to fix.
We might want to remove the versions altogether instead of taking on the burden of keeping them maintained.

I'm on board with not listing the version numbers and just telling people to look at package.cordovabuild.json and package.serve.json.

Overall I think this paragraph is 'fluff' that gets in the way and can be removed

shankari · 2023-09-23T20:58:21Z

README.md

 ---
+
+## 1. Creating logos


I am not sure why "creating logos" is the first step. If you are working on the UI only, you don't have to update the logos. Even if you are working on the plugins, you do not need to create logos unless you are building your own app. Why does a UI-only developer even need to care about the logos?

README.md

shankari · 2023-09-24T03:41:13Z

README.md

 ```

 ### Activation (after install, and in every new shell)
-
+2. Run this to activate 


why do you need this given that there is exactly one command to run?

This comment has not been addressed

shankari · 2023-09-24T03:44:01Z

README.md

+2. Change the devapp connection URL to one of these (e.g. 192.168.162.1:3000) and press "Connect"
+3. The app will now display the version of e-mission app that is in your local directory


Markdown will autonumber lists that start with 1., 1., 1.
It is better not to number them manually to avoid mistakes.

See the existing README for validation

This comment has not been addressed

shankari · 2023-09-24T04:25:52Z

README.md

+npm run build-dev-android
+```
+Your output should look something like this:
+![Build Successful Message screenshot](Build_ss.png)


As I have said before, image-based screenshots are not the best way to represent text-based outputs. Please replace with text, like the other expected outputs.

shankari · 2023-09-24T04:26:42Z

README.md

---
+are available in the [e-mission-server README](https://github.com/e-mission/e-mission-server/blob/master/README.md).
+
+In order to make end to end testing easy, if the local server is started on a HTTP (versus HTTPS port), it is in development mode.  By default, the phone app connects to the local server (localhost on iOS, [10.0.2.2 on android](https://stackoverflow.com/questions/5806220/how-to-connect-to-my-http-localhost-web-server-from-android-emulator-in-eclips)) with the `prompted-auth` authentication method. To connect to a different server, or to use a different authentication method, you need to create a `www/json/connectionConfig.json` file. More details on configuring authentication [can be found in the docs](https://github.com/e-mission/e-mission-docs/blob/master/docs/install/configuring_authentication.md).


a fair amount of this is obsolete given the dynamic config. Did you check with @JGreenlee?

Is connectionConfig.json used at all anymore?

After the master_for_platform -> master change, I don't think so.

$ grep -r connectionConfig www/js

doesn't find any usages.

We should test removing it completely (in addition to fixing the README)

shankari · 2023-09-24T04:28:13Z

README.md

+
+In order to make end to end testing easy, if the local server is started on a HTTP (versus HTTPS port), it is in development mode.  By default, the phone app connects to the local server (localhost on iOS, [10.0.2.2 on android](https://stackoverflow.com/questions/5806220/how-to-connect-to-my-http-localhost-web-server-from-android-emulator-in-eclips)) with the `prompted-auth` authentication method. To connect to a different server, or to use a different authentication method, you need to create a `www/json/connectionConfig.json` file. More details on configuring authentication [can be found in the docs](https://github.com/e-mission/e-mission-docs/blob/master/docs/install/configuring_authentication.md).
+
+One advantage of using `skip` authentication in development mode is that any user email can be entered without a password. Developers can use one of the emails that they loaded test data for in step (3) above. So if the test data loaded was with `-u shankari@eecs.berkeley.edu`, then the login email for the phone app would also be `shankari@eecs.berkeley.edu`.


This is also now different given the one-step opcode. Have you actually successfully run end-to-end with these instructions?

shankari · 2023-09-24T04:28:43Z

README.md

+xcode-select --install
+```
+
+2. Creating Logos


why is this here?

why is this here?

From my experience upgrading from Monterey to Ventura (on two devices), I noticed that the update removes the CLI tools. Given that this has nothing to do with e-mission-phone and more to do with Mac devices, I can remove it in the next PR. I only added this because ease I remember you mentioning something regarding Mac specific and thought it would be useful.

shankari · 2023-09-24T04:29:26Z

README.md

+git checkout master
+```
+```
+git pull upstream master
+```
+```
+git push origin master
+```
 ```
+git branch -d <branch>
+```
+


Why are these in separate code blocks?

niccolopaganini · 2023-09-24T18:09:32Z

Changes needed to be done for the next PR update:

Remove manual numbering
Remove screenshot and replace with text
Verify the one step OPcode process for end-to-end testing
Remove "Tested On" as it makes no sense
There is still an <insert_link> which you need to fix.

We might want to remove the versions altogether instead of taking on the burden of keeping them maintained.

Remove it
6. Update CI for Android SDK install
7. Installation of JDK ought to be fine. Adding the PATH to be removed from the README

If you have setup failures, please compare the configuration in the passing CI builds with your configuration. That is almost certainly the source of the error.

Add it
8. Speaking of CI builds, remove the versions numbers and make sure to point anyone to the CI for reference.

niccolopaganini · 2023-09-24T23:47:50Z

"Formatting" as a whole:

1. Activation for the plugins

I think you should've been more clear but it is evident that I what I tried to achieve failed. I was basically trying to capture the repeatable process separately

2. Code without the $ sign:
I removed it for the sole reason being that when speaking with Shrestrha (I could not find his username, I will modify the comment later), the copy button of the GitHub also copies the $, which is why I deviated from the convention. That said, on a high level, I searched for a couple of repos in GitHub and none seem to follow. That said, which one would you advice moving forward:
- This one (as per convention):

$ xyz

OR

This one (to accommodate the convention but can be easily copied from the README): $xyz
OR

This one (the current way):

xyz

4. > Why are these in separate code blocks?

Same as above; easy to copy-paste the commands -

*The above is for convenience*

5. Structure
From speaking with both Shrestha & Grassi, I got the impression/ feedback from them that it was kind of confusing, and Shrestha explicitly stated a contents section in the README would be nice. What do you think @shankari, do you think it will be helpful. Although everything is tied in the app, the e-mission-phone repo's sections are independent; which is why I inserted markdown anchors. If it is not correct, I can keep the anchor links and restore it to the original structure.

shankari · 2023-09-25T04:40:41Z

That said, on a high level, I searched for a couple of repos in GitHub and none seem to follow.

Which repos? What does "none seem to follow" mean concretely?

From speaking with both Shrestha & Grassi, I got the impression/ feedback from them that it was kind of confusing, and Shrestha explicitly stated a contents section in the README would be nice. What do you think @shankari, do you think it will be helpful. Although everything is tied in the app, the e-mission-phone repo's sections are independent; which is why I inserted markdown anchors. If it is not correct, I can keep the anchor links and restore it to the original structure.

I don't disagree that it is confusing, and I don't object to a ToC. I just don't think that changing the structure to mix up the UI-only section and the native code section makes it less confusing. What is the concrete feedback that drives that change in structure.

Note also that you have only interviewed people who are working on native code, not on UI-only changes.

I have added detailed feedback on where I think that the structure changes are incorrect. I am not sure what else you are looking for.

Removed manual numbering and updated markdown anchors

…on 3. Revised formatting for activation sub-section -> for better clarity

shankari

I don't see that all my comments have been addressed. Please address or provide justification before asking for re-review.

README.md

niccolopaganini · 2023-09-28T23:07:26Z

- if you are not on the most recent version of OSX: 'homebrew'
I would like to as of how to proceed with this as Mac OS Sonoma was recently released. I tried building it today with my personal computer and I was able to build it successfully.

Shall I state it as follows:

- if you are not on Mac OS Ventura/ Sonoma: 'homebrew'

shankari · 2023-09-29T05:43:08Z

Shall I state it as follows:

if you are not on Mac OS Ventura/ Sonoma: 'homebrew'

@niccolopaganini I think we have made it fairly clear that we don't want to put any versions into the README because they get obsolete very quickly.
#1037 (comment)
#1037 (comment)

We should just always use homebrew. Remove the part about the "most recent version"
Did you check with @iantei on the errors that he has been having?

1. Removed instructions on JAVA_HOME as it is no longer required 2. Quality of life changes

1. Changed the content section 2. Updated links in "Contributing" section 3. Updated "end to end testing" section

1. Updated buttons to point to the right CI 2. Removed point no. 4 in troubleshooting (it was empty)

Made sure the links for the buttons are correct

niccolopaganini · 2023-09-30T02:59:58Z

Changes mentioned that are taken care of:

JAVA_HOME section to be removed
'Update links' that point to the CI
Remove "floating links"
Updated sections:
- end-to-end testing

shankari

@niccolopaganini could you please just revert everything except:

table of contents
npm run instead of npx
removing versions
new CI badge for prereq-install
resolve merge conflict

That is all that is currently broken. We need to fix the broken stuff right now.

I sympthize with your desire to rewrite the README, but it would be better to postpone that until a time when:

it is not broken
you understand the project better

shankari · 2023-09-30T03:55:33Z

Build_ss.png

this file is not used anywhere

Let me go ahead and remove it

shankari · 2023-09-30T04:00:13Z

README.md

+#### 1. [Updating the UI only](#updating-the-ui-only)
+#### 2. [Updating the e-mission-* plugins or adding new plugins](#updating-the-e-mission--plugins-or-adding-new-plugins)  
+#### 3. [Creating logos](#creating-logos) 
+#### 4. [End to End Testing](#end-to-end-testing)


why is end-to-end testing step 4? The common case so far is UI development. As I said before:
#1037 (comment)
it should be before the native build

shankari · 2023-09-30T04:01:37Z

README.md


-:sparkles: This has now been upgraded to cordova android@9.0.0 and iOS@6.0.1 ([details](https://github.com/e-mission/e-mission-docs/issues/554)). It has also been upgraded to [android API 29](https://github.com/e-mission/e-mission-phone/pull/707/), [cordova-lib@10.0.0 and the most recent node and npm versions](https://github.com/e-mission/e-mission-phone/pull/708)It also now supports CI, so we should not have any build issues in the future. The limitations from the [previous upgrade](https://github.com/e-mission/e-mission-docs/issues/519) have all been resolved. This should be ready to build out of the box, after all the configuration files are changed.
+:sparkles: This has now been upgraded to cordova android@12.0.0 and iOS@6.2.0. It has also been upgraded to the **latest Android & iOS versions**, **cordova-lib@10.0.0 and the most recent node and npm versions**. It also now supports CI, so we should not have any build issues in the future. __This should be ready to build out of the box.__


Why have the versions not been removed?
#1037 (comment)

shankari · 2023-09-30T04:03:26Z

README.md

---
+✨ We constantly upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date. 
+
+For the latest versions, refer [`package.cordovabuild.json`](https://github.com/e-mission/e-mission-phone/blob/fce117ff859abd995613bd405dbc7d27c703b09b/package.cordovabuild.json)


having this include a permalink defeats the purpose of using it to check the latest links

Ah shoot. Would relative path be a good substitute?

shankari · 2023-09-30T04:04:54Z

README.md

+## Contents
+#### 1. [Updating the UI only](#updating-the-ui-only)
+#### 2. [Updating the e-mission-* plugins or adding new plugins](#updating-the-e-mission--plugins-or-adding-new-plugins)  
+#### 3. [Creating logos](#creating-logos) 


why is creating logos above end to end testing? Creating new logos is a very very unusual task

To add on to this, why is the ToC not consistent with the section order?

shankari · 2023-09-30T04:11:04Z

README.md

+```
+"connectUrl": "https://nrel-commute-openpath.nrel.gov/api/"
+```
+More details on configuring authentication [can be found in the docs](https://github.com/e-mission/e-mission-docs/blob/master/docs/install/configuring_authentication.md).


large parts of this document are obsolete.The phone auth is now based completely on the dynamic config.

shankari · 2023-09-30T04:13:53Z

README.md

- Java 11. Tested with [OpenJDK 11 (Temurin) using AdoptOpenJDK](https://adoptopenjdk.net/releases.html).
- android SDK; install manually or use setup script below. Note that you only need to run this once **per computer**.
+- Java 17. Tested with [OpenJDK 17 (Temurin) using AdoptOpenJDK](https://adoptium.net). 
+- Always use [homebrew](https://brew.sh) in addition to CLI


"in addition to CLI"? What does that mean?

This has not been addressed

shankari · 2023-09-30T04:15:39Z

README.md

+Activating nvm
+Using version 19.5.0
+Now using node v19.5.0 (npm v9.3.1)
+npm version = 9.3.1
+Adding cocoapods to the path


This has not been addressed

shankari · 2023-09-30T04:16:35Z

README.md

+xcode-select --install
+```
+
+__2. Creating Logos__


why is "Creating logos" here? The rest of the text has nothing do to with logos

shankari · 2023-09-30T04:17:47Z

README.md


-Once I merge the pull request, pull the changes to your fork and delete the branch
+---
+### Troubleshooting


This is troubleshooting for the native build - why has it been moved to the bottom?

This has not been addressed

niccolopaganini · 2023-09-30T19:40:30Z

I sympthize with your desire to rewrite the README, but it would be better to postpone that until a time when:

it is not broken

you understand the project better

I read your above comments and I will go ahead and restore the original architecture. I am not sure why the changes you said are not being reflected... Let me edit the following changes required and then push the local changes again.

I am not sure what happened during the restoration process that the changes were not saved/ are visible. Let me go ahead and restore it to the original form.

niccolopaganini · 2023-09-30T19:49:27Z

As for end-to-end testing, I must have understood it wrong when @JGreenlee conveyed the changes that had taken place. Let me re-clarify and update it asap

1. Removed manual numbering 2. Removed any form of version numbers 3. Removed certain Permalinks and swapped it with relative PATHs 4. Made sure the structure is same

JGreenlee · 2023-10-01T05:19:38Z

@niccolopaganini I showed you nrel-commute.nrel-op.json as one example of a dynamic config file.

Every ongoing study/program has its own config file. This file could be called anything and could include any number of the supported config options.

If you skim through all the different config files in the directory I showed (https://github.com/e-mission/nrel-openpath-deploy-configs/tree/main/configs) this should become clearer.

Each config file has different values in the server field. Each one has a different URL, reflecting the endpoint where the server for that study is running.

And a few of the configs (the dev-emulator-* ones) just don't have a server field at all. In this case the server is assumed to be running on the same machine (localhost)

So the relevance to the README is that if someone has e-mission-server running at a specific URL or IP address, and they want to connect to it, they would have to specify that URL or IP in the server field of their dynamic config file.

niccolopaganini · 2023-10-01T05:20:45Z

Ah I see... totally misconstrued what you said. Let me go through them and update the README asap

iter 1 - fix end to end testing

shankari

I still don't see the CI for the prereq install.
There are also several unaddressed comments from the previous reviews.

It takes me close to an hour for each such review, which is somewhat pointless if the code is not ready to merge. So I am not going to do any more reviews until I see that at least the CI has been added.

shankari · 2023-10-01T22:17:25Z

Build_ss.png

This file is still here

shankari · 2023-10-01T22:19:15Z

README.md


-Additional Documentation
---
+✨ We constantly upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date. 


We don't constantly upgrade the repo. Not sure what you mean by "the CI will be up-to-date".

Suggested change

✨ We constantly upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date.

✨ We periodically upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date.

README.md

shankari · 2023-10-01T22:20:30Z

README.md

+## Contents
+#### 1. [Updating the UI only](#updating-the-ui-only)
+#### 2. [Updating the e-mission-* plugins or adding new plugins](#updating-the-e-mission--plugins-or-adding-new-plugins)  
+#### 3. [Creating logos](#creating-logos) 


To add on to this, why is the ToC not consistent with the section order?

shankari · 2023-10-01T22:21:31Z

README.md

@@ -1,75 +1,87 @@
-e-mission phone app
--------------------
+# [e-mission phone app](https://github.com/e-mission/e-mission-phone/tree/master)


why does this need to be a URL given that we are literally in the same repo?

shankari · 2023-10-01T22:46:36Z

README.md

+```
+export ANDROID_SDK_ROOT="/Users/<user_name>/Library/Android/sdk"
+```


We want to minimize manual copy-paste so these should go into the setup scripts instead.
We don't make people export anything else, so why this?
Please see my earlier comment this variable and JAVA_HOME.

This change has been pending for ~ 3 weeks, so I am fine with cleaning that up later.

shankari · 2023-10-01T22:47:09Z

README.md

- Java 11. Tested with [OpenJDK 11 (Temurin) using AdoptOpenJDK](https://adoptopenjdk.net/releases.html).
- android SDK; install manually or use setup script below. Note that you only need to run this once **per computer**.
+- Java 17. Tested with [OpenJDK 17 (Temurin) using AdoptOpenJDK](https://adoptium.net). 
+- Always use [homebrew](https://brew.sh) in addition to CLI


This has not been addressed

shankari · 2023-10-01T22:52:17Z

README.md

+If you have setup failures, please compare the configuration in the **passing CI
+builds** with your configuration. That is almost certainly the source of the error.
+
+__Export statements__


I don't think this warrants a bolded heading, especially when it is going to be removed anyway.

shankari · 2023-10-01T22:56:23Z

README.md

-
-Troubleshooting
---
- Make sure to use `npx ionic` and `npx cordova`. This is


why has this not been restored to its original place?

shankari · 2023-10-01T22:56:57Z

README.md


-Once I merge the pull request, pull the changes to your fork and delete the branch
+---
+### Troubleshooting


This has not been addressed

Not required anymore

- Better wordings

- Removed relative PATH for dynamic links as they don't seem to work

Modified the server to `server`

niccolopaganini · 2023-10-07T16:43:49Z

table of contents ✅

'Update links' that point to the CI ✅

npm run instead of npx ✅

…. Added CI badge 3. Plugins installation guide updated

README.md

JGreenlee · 2023-10-16T15:29:08Z

README.md

 ```
-$ npx cordova emulate ios
+npm run <ios>
+```
 AND/OR
-$ npx cordova emulate android
 ```
+npm run <android>
+```


It is not clear what <ios> and <android> are.

I think we should just give examples of the most common build scripts that we think will be used and list them out here.

npm run build (to build for both Android and iOS platforms)
npm run build-prod-android (to build for Android platform only)
npm run build-prod-ios (to build for iOS platform only)

Adding suggested changes Co-authored-by: shankari <shankari@eecs.berkeley.edu>

Committing suggested changes Co-authored-by: Jack Greenlee <JackAGreenlee@gmail.com>

Committing changes based on suggestions - Wording - Listing common build types

Reword the "Building the app" section to make it clearer what scripts are available and what they do

Remove the note about putting your own configuration files in the `/json` directory. We use the dynamic config now and encourage people to use it rather than making local code changes.

update outdated comments about connectionConfig.json to reflect that we don't use connectionConfig.json anymore; this is replaced by the 'server' field of the dynamic config. If not present, it uses localhost.

reword "Building the app"

make "Updating the UI only" a bit clearer: - you don't have to manually type the IP address in the devapp if it's on localhost - you do have to type it manually if it's on a different device, and you should make sure that it's on the same network

Remove the note about putting your own configuration files in the `/json` directory. We use the dynamic config now and encourage people to use it rather than making local code changes.

README.md

Committing suggested change Co-authored-by: K. Shankari <shankari@eecs.berkeley.edu>

Committing change based on suggestion (adding a link that points to Noel-openpath-deploy-configs)

README.md

niccolopaganini and others added 7 commits September 12, 2023 00:47

Updated README

7ca6fcd

1. Updated README like @JGreenlee mentioned to successfully build 2. Maintaining current version 3. Will update again after updating JDK in this intel machine

multiple changes: 1. structure/ flow 2. added contents section 3. upd…

6822c67

…ated certain resources

build successful screenshot

8ea506d

Update README.md

ed0e871

- Added hyperlink to the title - Added output screenshot

Update README.md

54656b8

- Updating markdown anchors

Update README.md

290dcc5

Updating markdown anchors as 4,5, & 6 weren't working

Update README.md

809fc5d

- touch up - fixed non-working markdown anchors

niccolopaganini mentioned this pull request Sep 23, 2023

README update for the changes undergone #1030

Closed

niccolopaganini changed the title ~~Label dashboard profile sept 2023~~ Updated README for Label dashboard profile sept 2023 Sep 23, 2023

shankari requested changes Sep 24, 2023

View reviewed changes

niccolopaganini and others added 2 commits September 25, 2023 00:29

Update README.md

b64ab62

Removed manual numbering and updated markdown anchors

1. Changed expected output for plugins 2. Removed tested on sub-secti…

72922cf

…on 3. Revised formatting for activation sub-section -> for better clarity

shankari requested changes Sep 25, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

niccolopaganini and others added 5 commits September 29, 2023 09:54

1. reformatted to the original structure 2. removed floating links

92ce707

Update README.md

3d5e99b

1. Removed instructions on JAVA_HOME as it is no longer required 2. Quality of life changes

Update README.md

03bc5c3

1. Changed the content section 2. Updated links in "Contributing" section 3. Updated "end to end testing" section

Update README.md

4d4d57d

1. Updated buttons to point to the right CI 2. Removed point no. 4 in troubleshooting (it was empty)

Update README.md

bbf18bd

Made sure the links for the buttons are correct

shankari requested changes Sep 30, 2023

View reviewed changes

Update README.md

4da919f

1. Removed manual numbering 2. Removed any form of version numbers 3. Removed certain Permalinks and swapped it with relative PATHs 4. Made sure the structure is same

Update README.md

e8cbcb5

iter 1 - fix end to end testing

shankari requested changes Oct 1, 2023

View reviewed changes

niccolopaganini added 4 commits October 2, 2023 08:53

Delete Build_ss.png

498e04e

Not required anymore

Update README.md

f4f3e68

- Better wordings

Update README.md

e1a6a32

- Removed relative PATH for dynamic links as they don't seem to work

Update README.md

693852d

Modified the server to `server`

Updated README file: Restored to the original - 1. Removed versions 2…

32596a8

…. Added CI badge 3. Plugins installation guide updated

JGreenlee requested changes Oct 16, 2023

View reviewed changes

niccolopaganini and others added 10 commits October 17, 2023 10:42

Update README.md

b6e3c65

Adding suggested changes Co-authored-by: shankari <shankari@eecs.berkeley.edu>

Update README.md

2b5735e

Committing suggested changes Co-authored-by: Jack Greenlee <JackAGreenlee@gmail.com>

Update README.md

cb954d1

Committing changes based on suggestions - Wording - Listing common build types

Apply suggestions from code review

4643949

Update README.md

685e1b6

Reword the "Building the app" section to make it clearer what scripts are available and what they do

Update README.md

047c2b7

Remove the note about putting your own configuration files in the `/json` directory. We use the dynamic config now and encourage people to use it rather than making local code changes.

Update README.md

5f9d189

update outdated comments about connectionConfig.json to reflect that we don't use connectionConfig.json anymore; this is replaced by the 'server' field of the dynamic config. If not present, it uses localhost.

Update README.md

c6dc2e5

reword "Building the app"

Update README.md

96d29e6

make "Updating the UI only" a bit clearer: - you don't have to manually type the IP address in the devapp if it's on localhost - you do have to type it manually if it's on a different device, and you should make sure that it's on the same network

Update README.md

079f4ea

Remove the note about putting your own configuration files in the `/json` directory. We use the dynamic config now and encourage people to use it rather than making local code changes.

JGreenlee approved these changes Oct 18, 2023

View reviewed changes

shankari requested changes Oct 19, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

niccolopaganini and others added 2 commits October 19, 2023 12:05

Update README.md

58a50a6

Committing suggested change Co-authored-by: K. Shankari <shankari@eecs.berkeley.edu>

Update README.md

9d3965a

Committing change based on suggestion (adding a link that points to Noel-openpath-deploy-configs)

shankari reviewed Oct 19, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

clarify link to repo

ca0d569

shankari merged commit 26f30d7 into e-mission:master Oct 20, 2023
4 checks passed

shankari mentioned this pull request Oct 21, 2023

Scripts and documentation for building the app are outdated e-mission/e-mission-docs#958

Closed


		:sparkles: This has now been upgraded to cordova android@9.0.0 and iOS@6.0.1 ([details](https://github.com/e-mission/e-mission-docs/issues/554)). It has also been upgraded to [android API 29](https://github.com/e-mission/e-mission-phone/pull/707/), [cordova-lib@10.0.0 and the most recent node and npm versions](https://github.com/e-mission/e-mission-phone/pull/708)It also now supports CI, so we should not have any build issues in the future. The limitations from the [previous upgrade](https://github.com/e-mission/e-mission-docs/issues/519) have all been resolved. This should be ready to build out of the box, after all the configuration files are changed.
		:sparkles: This has now been upgraded to cordova android@12.0.0 and iOS@6.2.0. It has also been upgraded to [android API 33 and the latest iOS versions](https://github.com/e-mission/e-mission-docs/issues/934), [cordova-lib@10.0.0 and the most recent node and npm versions](<insert link>). It also now supports CI, so we should not have any build issues in the future. __This should be ready to build out of the box.__

		2. Change the devapp connection URL to one of these (e.g. 192.168.162.1:3000) and press "Connect"
		3. The app will now display the version of e-mission app that is in your local directory


		In order to make end to end testing easy, if the local server is started on a HTTP (versus HTTPS port), it is in development mode. By default, the phone app connects to the local server (localhost on iOS, [10.0.2.2 on android](https://stackoverflow.com/questions/5806220/how-to-connect-to-my-http-localhost-web-server-from-android-emulator-in-eclips)) with the `prompted-auth` authentication method. To connect to a different server, or to use a different authentication method, you need to create a `www/json/connectionConfig.json` file. More details on configuring authentication [can be found in the docs](https://github.com/e-mission/e-mission-docs/blob/master/docs/install/configuring_authentication.md).

		One advantage of using `skip` authentication in development mode is that any user email can be entered without a password. Developers can use one of the emails that they loaded test data for in step (3) above. So if the test data loaded was with `-u shankari@eecs.berkeley.edu`, then the login email for the phone app would also be `shankari@eecs.berkeley.edu`.

	✨ We constantly upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date.
	✨ We periodically upgrade the repo to the latest cordova versions of android, iOS, cordova-lib, and the most recent node and npm versions. The CI will be up-to-date.

Updated README for Label dashboard profile sept 2023 #1037

Updated README for Label dashboard profile sept 2023 #1037

Conversation

niccolopaganini commented Sep 23, 2023

shankari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niccolopaganini commented Sep 24, 2023 • edited Loading

niccolopaganini commented Sep 24, 2023

"Formatting" as a whole:

shankari commented Sep 25, 2023 • edited Loading

shankari left a comment

Choose a reason for hiding this comment

niccolopaganini commented Sep 28, 2023 • edited Loading

shankari commented Sep 29, 2023 • edited Loading

niccolopaganini commented Sep 30, 2023

shankari left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niccolopaganini commented Sep 30, 2023 • edited Loading

niccolopaganini commented Sep 30, 2023

JGreenlee commented Oct 1, 2023

niccolopaganini commented Oct 1, 2023

shankari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niccolopaganini commented Oct 7, 2023

Choose a reason for hiding this comment

niccolopaganini commented Sep 24, 2023 •

edited

Loading

shankari commented Sep 25, 2023 •

edited

Loading

niccolopaganini commented Sep 28, 2023 •

edited

Loading

shankari commented Sep 29, 2023 •

edited

Loading

shankari left a comment •

edited

Loading

niccolopaganini commented Sep 30, 2023 •

edited

Loading