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

README update for the changes undergone #1030

Conversation

niccolopaganini
Copy link
Contributor

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

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

@shankari shankari left a comment

Choose a reason for hiding this comment

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

Please indicate "Testing done"
"If it is not tested, it is broken".

In particular, I would like to see if the instructions work without activation (see comment below)

README.md Outdated
Comment on lines 144 to 149
Make sure you switch to the "label_dashboard_profile_sept_2023" branch
```
git checkout label_dashboard_profile_sept_2023
```

```
Copy link
Contributor

Choose a reason for hiding this comment

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

The label_dashboard_profile_sept_2023 branch is a temporary one that we are using for the current release.
It will be deleted after the release is moved to production.

Comment on lines -159 to -166
### Activation (after install, and in every new shell)

```
$ source setup/activate_native.sh
```

### Activation (after install, and in every new shell)

Copy link
Contributor

@shankari shankari Sep 12, 2023

Choose a reason for hiding this comment

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

why did you remove this? I believe we do need to activate after install. How did you test this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to build the app successfully. Let me verify the above again just to be sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

were you able to build the app starting in a new terminal and following the instructions in the README exactly?
When you check, please show, don't tell.

README.md Outdated
@@ -174,10 +171,15 @@ If connecting to a development server over http, make sure to turn on http suppo

### Run in the emulator

Pick a version and execute the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that "type of build" == "version"

Suggested change
Pick a version and execute the following:
Pick a type of build and execute the following:

You should also indicate how people can find the valid "type of build" supported.

@niccolopaganini niccolopaganini changed the title Updated README README update for the changes undergone Sep 22, 2023
@niccolopaganini
Copy link
Contributor Author

Ugh.. I lost all changes I had made to the README while pushing the changes to my repo. I will work on the README once I get some rest. Apologies (not sure what happened)

niccolopaganini and others added 6 commits September 23, 2023 13:13
- 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
@niccolopaganini
Copy link
Contributor Author

opened a new PR #1037

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